Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DYN-5733 fix language CLI argument #15739

Merged
merged 4 commits into from
Jan 7, 2025
Merged

DYN-5733 fix language CLI argument #15739

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5ef1ba
fix language CLI argument
chubakueno Dec 31, 2024
c0169ee
external public interface compatibility
chubakueno Jan 3, 2025
6494d89
refactor MakeModel to use PrepareModel
chubakueno Jan 3, 2025
d909e2e
make PrepareModel private
chubakueno Jan 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 46 additions & 36 deletions src/DynamoApplications/StartupUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,36 @@ public static void PreloadShapeManager(ref string geometryFactoryPath, ref strin
geometryFactoryPath = preloader.GeometryFactoryPath;
preloaderLocation = preloader.PreloaderLocation;
}

/// <summary>
/// Use this overload to construct a DynamoModel in CLI context when the location of ASM to use is known, host analytics info is known and you want to set data paths.
/// </summary>
/// <param name="asmPath">Path to directory containing geometry library binaries</param>
/// <param name="userDataFolder">Path to be used by PathResolver for UserDataFolder</param>
/// <param name="commonDataFolder">Path to be used by PathResolver for CommonDataFolder</param>
/// <param name="info">Host analytics info specifying Dynamo launching host related information.</param>
/// <returns></returns>
public static DynamoModel MakeCLIModel(string asmPath, string userDataFolder, string commonDataFolder, HostAnalyticsInfo info = new HostAnalyticsInfo())
public static DynamoModel PrepareModel(
Copy link
Member

@mjkkirschner mjkkirschner Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot break these public api signatures of a public class.

Copy link
Contributor

@RobertGlobant20 RobertGlobant20 Jan 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chubakueno I guess a better option should be creating an overloaded for public static DynamoModel MakeModel which receives string CLIlocale as parameter so in this way you won't need to modify the existing public method (I think default argument is also NOT an option).

Copy link
Contributor Author

@chubakueno chubakueno Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, though there are now two(previously there already was one) unused overloads of the methods.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chubakueno I can see that now we have three MakeModel() methods and only two of them are calling PrepareModel(), is possible to also use PrepareModel() in the missing one?
image

Copy link
Contributor Author

@chubakueno chubakueno Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Behavior before was not identical to the other overloads so PrepareModel wasn't an equivalent refactor (no calls to AssignHostPathAndIPathResolver, SetUICulture, OnDetectLanguage, OnRequestUpdateLoadBarStatus), but that MakeModel is only used by integration tests and the lack of those calls is likelier than not to be an unintentional omission so i think it's a good idea that all exhibit the same behavior.

string cliLocale,
string asmPath,
bool noNetworkMode,
HostAnalyticsInfo analyticsInfo,
bool cliMode = true,
string userDataFolder = "",
string commonDataFolder = "",
bool serviceMode = false)
{
var normalizedCLILocale = string.IsNullOrEmpty(cliLocale) ? null : cliLocale;
IPathResolver pathResolver = CreatePathResolver(false, string.Empty, string.Empty, string.Empty);
PathManager.Instance.AssignHostPathAndIPathResolver(string.Empty, pathResolver);
DynamoModel.SetUICulture(normalizedCLILocale ?? PreferenceSettings.Instance.Locale);
DynamoModel.OnDetectLanguage();

// Preload ASM and display corresponding message on splash screen
DynamoModel.OnRequestUpdateLoadBarStatus(new SplashScreenLoadEventArgs(Resources.SplashScreenPreLoadingAsm, 10));
var isASMloaded = PreloadASM(asmPath, out string geometryFactoryPath, out string preloaderLocation);
var model = StartDynamoWithDefaultConfig(true, userDataFolder, commonDataFolder, geometryFactoryPath, preloaderLocation, false, info);
var model = StartDynamoWithDefaultConfig(
CLImode: cliMode,
userDataFolder: userDataFolder,
commonDataFolder: commonDataFolder,
geometryFactoryPath: geometryFactoryPath,
preloaderLocation: preloaderLocation,
noNetworkMode: noNetworkMode,
info: analyticsInfo,
isServiceMode: serviceMode,
cliLocale: normalizedCLILocale
);
model.IsASMLoaded = isASMloaded;
return model;
}
Expand All @@ -193,18 +208,15 @@ public static void PreloadShapeManager(ref string geometryFactoryPath, ref strin
/// <returns></returns>
public static DynamoModel MakeCLIModel(CommandLineArguments cmdLineArgs)
{
var asmPath = String.IsNullOrEmpty(cmdLineArgs.ASMPath) ? string.Empty : cmdLineArgs.ASMPath;
IPathResolver pathResolver = CreatePathResolver(false, string.Empty, string.Empty, string.Empty);
PathManager.Instance.AssignHostPathAndIPathResolver(string.Empty, pathResolver);
DynamoModel.SetUICulture(PreferenceSettings.Instance.Locale);
DynamoModel.OnDetectLanguage();

// Preload ASM and display corresponding message on splash screen
DynamoModel.OnRequestUpdateLoadBarStatus(new SplashScreenLoadEventArgs(Resources.SplashScreenPreLoadingAsm, 10));
var isASMloaded = PreloadASM(asmPath, out string geometryFactoryPath, out string preloaderLocation);
var model = StartDynamoWithDefaultConfig(true, cmdLineArgs.UserDataFolder, cmdLineArgs.CommonDataFolder,
geometryFactoryPath, preloaderLocation, cmdLineArgs.NoNetworkMode, cmdLineArgs.AnalyticsInfo, cmdLineArgs.ServiceMode);
model.IsASMLoaded = isASMloaded;
var asmPath = string.IsNullOrEmpty(cmdLineArgs.ASMPath) ? string.Empty : cmdLineArgs.ASMPath;
var model = PrepareModel(
cliLocale: cmdLineArgs.Locale,
asmPath: asmPath,
noNetworkMode: cmdLineArgs.NoNetworkMode,
analyticsInfo: cmdLineArgs.AnalyticsInfo,
userDataFolder: cmdLineArgs.UserDataFolder,
commonDataFolder: cmdLineArgs.CommonDataFolder,
serviceMode: cmdLineArgs.ServiceMode);
return model;
}

Expand All @@ -228,23 +240,19 @@ public static DynamoModel MakeModel(bool CLImode, string asmPath = "", string ho
/// Use this overload to construct a DynamoModel when the location of ASM to use is known and host analytics info is known.
/// </summary>
/// <param name="CLImode">CLI mode starts the model in test mode and uses a separate path resolver.</param>
/// <param name="CLIlocale">CLI argument to force dynamo locale</param>
/// <param name="noNetworkMode">Option to initialize Dynamo in no-network mode</param>
/// <param name="asmPath">Path to directory containing geometry library binaries</param>
/// <param name="info">Host analytics info specifying Dynamo launching host related information.</param>
/// <returns></returns>
public static DynamoModel MakeModel(bool CLImode, bool noNetworkMode, string asmPath = "", HostAnalyticsInfo info = new HostAnalyticsInfo())
public static DynamoModel MakeModel(bool CLImode, string CLIlocale, bool noNetworkMode, string asmPath = "", HostAnalyticsInfo info = new HostAnalyticsInfo())
{
IPathResolver pathResolver = CreatePathResolver(false, string.Empty, string.Empty, string.Empty);
PathManager.Instance.AssignHostPathAndIPathResolver(string.Empty, pathResolver);
DynamoModel.SetUICulture(PreferenceSettings.Instance.Locale);
DynamoModel.OnDetectLanguage();

// Preload ASM and display corresponding message on splash screen
DynamoModel.OnRequestUpdateLoadBarStatus(new SplashScreenLoadEventArgs(Resources.SplashScreenPreLoadingAsm, 10));
var isASMloaded = PreloadASM(asmPath, out string geometryFactoryPath, out string preloaderLocation);
var model = StartDynamoWithDefaultConfig(CLImode, string.Empty, string.Empty, geometryFactoryPath,
preloaderLocation, noNetworkMode, info);
model.IsASMLoaded = isASMloaded;
var model = PrepareModel(
cliLocale: CLIlocale,
asmPath: asmPath,
noNetworkMode: noNetworkMode,
analyticsInfo: info,
cliMode: CLImode);
return model;
}

Expand Down Expand Up @@ -322,7 +330,8 @@ private static DynamoModel StartDynamoWithDefaultConfig(bool CLImode,
string preloaderLocation,
bool noNetworkMode,
HostAnalyticsInfo info = new HostAnalyticsInfo(),
bool isServiceMode = false)
bool isServiceMode = false,
string cliLocale = null)
{

var config = new DynamoModel.DefaultStartConfiguration
Expand All @@ -337,6 +346,7 @@ private static DynamoModel StartDynamoWithDefaultConfig(bool CLImode,
IsServiceMode = isServiceMode,
Preferences = PreferenceSettings.Instance,
NoNetworkMode = noNetworkMode,
CLILocale = cliLocale,
//Breaks all Lucene calls. TI enable this would require a lot of refactoring around Lucene usage in Dynamo.
//IsHeadless = CLImode
};
Expand Down
9 changes: 8 additions & 1 deletion src/DynamoCore/Models/DynamoModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ public static string Version
/// </summary>
internal bool NoNetworkMode { get; }

/// <summary>
/// Locale forced by cli arguments
/// </summary>
internal string CLILocale { get; set; }

/// <summary>
/// The path manager that configures path information required for
/// Dynamo to function properly. See IPathManager interface for more
Expand Down Expand Up @@ -589,6 +594,7 @@ public struct DefaultStartConfiguration : IStartConfiguration
/// CLIMode indicates if we are running in DynamoCLI or DynamoWPFCLI mode.
/// </summary>
public bool CLIMode { get; set; }
public string CLILocale { get; set; }
}

/// <summary>
Expand Down Expand Up @@ -635,6 +641,7 @@ protected DynamoModel(IStartConfiguration config)
DefaultPythonEngine = defaultStartConfig.DefaultPythonEngine;
CLIMode = defaultStartConfig.CLIMode;
IsServiceMode = defaultStartConfig.IsServiceMode;
CLILocale = defaultStartConfig.CLILocale;
}

if (config is IStartConfigCrashReporter cerConfig)
Expand Down Expand Up @@ -685,7 +692,7 @@ protected DynamoModel(IStartConfiguration config)

if (PreferenceSettings != null)
{
SetUICulture(PreferenceSettings.Locale);
SetUICulture(CLILocale ?? PreferenceSettings.Locale);
PreferenceSettings.PropertyChanged += PreferenceSettings_PropertyChanged;
PreferenceSettings.MessageLogged += LogMessage;
}
Expand Down
2 changes: 2 additions & 0 deletions src/DynamoCore/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1813,6 +1813,8 @@ Dynamo.Models.DynamoModel.CurrentWorkspace.set -> void
Dynamo.Models.DynamoModel.DefaultStartConfiguration
Dynamo.Models.DynamoModel.DefaultStartConfiguration.AuthProvider.get -> Greg.IAuthProvider
Dynamo.Models.DynamoModel.DefaultStartConfiguration.AuthProvider.set -> void
Dynamo.Models.DynamoModel.DefaultStartConfiguration.CLILocale.get -> string
Dynamo.Models.DynamoModel.DefaultStartConfiguration.CLILocale.set -> void
Dynamo.Models.DynamoModel.DefaultStartConfiguration.CLIMode.get -> bool
Dynamo.Models.DynamoModel.DefaultStartConfiguration.CLIMode.set -> void
Dynamo.Models.DynamoModel.DefaultStartConfiguration.Context.get -> string
Expand Down
4 changes: 3 additions & 1 deletion src/DynamoSandbox/DynamoCoreSetup.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class DynamoCoreSetup
private readonly string ASMPath;
private readonly HostAnalyticsInfo analyticsInfo;
private readonly bool noNetworkMode;
private readonly string CLILocale;
private const string sandboxWikiPage = @"https://github.com/DynamoDS/Dynamo/wiki/How-to-Utilize-Dynamo-Builds";
private DynamoViewModel viewModel = null;

Expand Down Expand Up @@ -59,6 +60,7 @@ public DynamoCoreSetup(string[] args)
// HostVersion = new Version(3, 3, 0),
//};
noNetworkMode = cmdLineArgs.NoNetworkMode;
CLILocale = cmdLineArgs.Locale;
}

public void RunApplication(Application app)
Expand Down Expand Up @@ -140,7 +142,7 @@ public void RunApplication(Application app)
private void LoadDynamoView()
{
DynamoModel model;
model = StartupUtils.MakeModel(false, noNetworkMode, ASMPath ?? string.Empty, analyticsInfo);
model = StartupUtils.MakeModel(false, CLILocale, noNetworkMode, ASMPath ?? string.Empty, analyticsInfo);
model.CERLocation = CERLocation;

viewModel = DynamoViewModel.Start(
Expand Down
Loading