-
-
Notifications
You must be signed in to change notification settings - Fork 395
Allow for an option to close other outputs for all scripts in pyrevit settings #2793
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.
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
base: develop
Are you sure you want to change the base?
Changes from 1 commit
d48b37b
ef4b916
a05cfa7
7a13621
a2c9c2b
0c3b43c
683bbc3
b1a81a2
88f13ff
c61c752
2be7743
1f188ea
b231f2d
1820cac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,12 @@ public enum PyRevitLogLevels | |
Debug | ||
} | ||
|
||
public enum OutputCloseMode | ||
{ | ||
CurrentCommand, | ||
CloseAll | ||
} | ||
|
||
public static class PyRevitConfigs | ||
{ | ||
private static readonly Logger logger = LogManager.GetCurrentClassLogger(); | ||
|
@@ -505,6 +511,45 @@ public static void SetLoadBetaTools(bool state) | |
cfg.SetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsLoadBetaKey, state); | ||
} | ||
|
||
// close other outputs config | ||
public static bool GetCloseOtherOutputs() | ||
{ | ||
var cfg = GetConfigFile(); | ||
var status = cfg.GetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOtherOutputsKey); | ||
return status != null ? bool.Parse(status) : PyRevitConsts.ConfigsCloseOtherOutputsDefault; | ||
} | ||
|
||
public static void SetCloseOtherOutputs(bool state) | ||
{ | ||
var cfg = GetConfigFile(); | ||
cfg.SetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOtherOutputsKey, state); | ||
} | ||
|
||
public static OutputCloseMode GetCloseOutputMode() | ||
{ | ||
var cfg = GetConfigFile(); | ||
var raw = cfg.GetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOutputModeKey); | ||
|
||
var s = (raw ?? PyRevitConsts.ConfigsCloseOutputModeDefault).Trim().Trim('"', '\''); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dosymep is there a better way than trimming the config value ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I remember, you don't need to trim it yourself, it should do it automatically There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The string trimming logic with both quotation marks seems unusual and may indicate a configuration parsing issue. The standard pyRevit configuration pattern doesn't typically require quote trimming. Consider whether this complexity is necessary or if it masks an underlying issue. var s = (raw ?? PyRevitConsts.ConfigsCloseOutputModeDefault).Trim(); actionsFeedback: Rate this comment to help me improve future code reviews:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove trim, the pyRevit library trimmed values by default There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. public static OutputCloseMode GetCloseOutputMode()
{
var cfg = GetConfigFile();
var raw = cfg.GetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOutputModeKey);
return Enum.TryParse<OutputCloseMode>(raw, true, out OutputCloseMode outCloseMode)
? outCloseMode
: OutputCloseMode.CurrentCommand;
} |
||
|
||
if (s.Equals(PyRevitConsts.ConfigsCloseOutputModeCloseAll, StringComparison.InvariantCultureIgnoreCase)) | ||
{ | ||
return OutputCloseMode.CloseAll; | ||
} | ||
|
||
return OutputCloseMode.CurrentCommand; | ||
} | ||
|
||
public static void SetCloseOutputMode(OutputCloseMode mode) | ||
{ | ||
var cfg = GetConfigFile(); | ||
var value = (mode == OutputCloseMode.CloseAll) | ||
? PyRevitConsts.ConfigsCloseOutputModeCloseAll | ||
: PyRevitConsts.ConfigsCloseOutputModeCurrentCommand; | ||
|
||
cfg.SetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOutputModeKey, value); | ||
} | ||
Comment on lines
+543
to
+551
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. public static void SetCloseOutputMode(OutputCloseMode outputCloseMode)
{
var cfg = GetConfigFile();
cfg.SetValue(PyRevitConsts.ConfigsCoreSection, PyRevitConsts.ConfigsCloseOutputModeKey, outputCloseMode.ToString());
} |
||
|
||
// cpythonengine | ||
public static int GetCpythonEngineVersion() | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,7 @@ public static class PyRevitConsts { | |
public const string ReleaseDirName = "release"; | ||
public const string SitePackagesDirName = "site-packages"; | ||
public const string PyRevitfileFilename = "PyRevitfile"; | ||
|
||
public const string NetFxFolder = "netfx"; | ||
public const string NetCoreFolder = "netcore"; | ||
|
||
|
@@ -85,6 +85,12 @@ public static class PyRevitConsts { | |
public const int ConfigsMinDriveSpaceDefault = 0; | ||
public const string ConfigsLoadBetaKey = "loadbeta"; | ||
public const bool ConfigsLoadBetaDefault = false; | ||
public const string ConfigsCloseOtherOutputsKey = "closeotheroutputs"; | ||
public const bool ConfigsCloseOtherOutputsDefault = false; | ||
public const string ConfigsCloseOutputModeKey = "closeoutputmode"; | ||
public const string ConfigsCloseOutputModeDefault = "currentcommand"; | ||
public const string ConfigsCloseOutputModeCurrentCommand = "currentcommand"; | ||
public const string ConfigsCloseOutputModeCloseAll = "closeall"; | ||
Comment on lines
+91
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can remove I think |
||
public const string ConfigsCPythonEngineKey = "cpyengine"; | ||
public const int ConfigsCPythonEngineDefault = 0; | ||
public const string ConfigsLocaleKey = "user_locale"; | ||
|
@@ -179,7 +185,7 @@ public static class PyRevitConsts { | |
public static string FindConfigFileInDirectory(string sourcePath) { | ||
var configMatcher = new Regex(ConfigsFileRegexPattern, RegexOptions.IgnoreCase); | ||
// capture exceptions that might occur getting the files under sourcePath | ||
// | ||
// | ||
try { | ||
if (CommonUtils.VerifyPath(sourcePath)) | ||
foreach (string subFile in Directory.GetFiles(sourcePath)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,31 +305,47 @@ def load_beta(self, state): | |
def output_close_others(self): | ||
"""Whether to close other output windows.""" | ||
return self.core.get_option( | ||
'close_other_outputs', | ||
default_value=False, | ||
CONSTS.ConfigsCloseOtherOutputsKey, | ||
default_value=CONSTS.ConfigsCloseOtherOutputsDefault, | ||
) | ||
|
||
@output_close_others.setter | ||
def output_close_others(self, state): | ||
self.core.set_option( | ||
'close_other_outputs', | ||
CONSTS.ConfigsCloseOtherOutputsKey, | ||
value=state | ||
) | ||
|
||
@property | ||
def output_close_mode(self): | ||
"""Output window closing mode: 'current_command' or 'close_all'.""" | ||
return self.core.get_option( | ||
'close_mode', | ||
default_value='current_command', | ||
def output_close_mode_enum(self): | ||
"""Output window closing mode as enum (CurrentCommand | CloseAll).""" | ||
value = self.core.get_option( | ||
CONSTS.ConfigsCloseOutputModeKey, | ||
default_value=CONSTS.ConfigsCloseOutputModeDefault, | ||
) | ||
if not value: | ||
value = CONSTS.ConfigsCloseOutputModeDefault | ||
|
||
@output_close_mode.setter | ||
def output_close_mode(self, mode): | ||
self.core.set_option( | ||
'close_mode', | ||
value=mode | ||
) | ||
value_lc = str(value).lower() | ||
|
||
if value_lc == str(CONSTS.ConfigsCloseOutputModeCloseAll).lower(): | ||
return PyRevit.OutputCloseMode.CloseAll | ||
else: | ||
return PyRevit.OutputCloseMode.CurrentCommand | ||
|
||
@output_close_mode_enum.setter | ||
def output_close_mode_enum(self, mode): | ||
"""Store string in INI, mapped from enum.""" | ||
if mode == PyRevit.OutputCloseMode.CloseAll: | ||
self.core.set_option( | ||
CONSTS.ConfigsCloseOutputModeKey, | ||
value=CONSTS.ConfigsCloseOutputModeCloseAll | ||
) | ||
else: | ||
self.core.set_option( | ||
CONSTS.ConfigsCloseOutputModeKey, | ||
value=CONSTS.ConfigsCloseOutputModeCurrentCommand | ||
Comment on lines
+336
to
+347
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @property
def output_close_mode(self):
"""Output window closing mode as enum (CurrentCommand | CloseAll)."""
value = self.core.get_option(
CONSTS.ConfigsCloseOutputModeKey,
default_value=CONSTS.ConfigsCloseOutputModeDefault,
)
if System.Enum.TryParse[PyRevit.OutputCloseMode](value, true, out PyRevit.OutputCloseMode output_close_mode):
return output_close_mode
return PyRevit.OutputCloseMode.CurrentCommand
@output_close_mode.setter
def output_close_mode(self, output_close_mode):
"""Store string in INI, mapped from enum."""
self.core.set_option(
CONSTS.ConfigsCloseOutputModeKey,
value=str(output_close_mode)
) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I renamed methods (remove |
||
) | ||
Comment on lines
320
to
348
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dosymep With enum, I had to deal with mapping somewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's good, I hope in the future I will rewrite all the code for working with settings, and there will be a more convenient option There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove only the enum suffix in name, I don't think users need to see it |
||
|
||
@property | ||
def cpython_engine_version(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
BeginInvoke
for UI marshalling may introduce unnecessary delay. Consider whether the console closing operation needs to be asynchronous or ifInvoke
(synchronous) would be more appropriate for immediate feedback.The current implementation may cause timing issues where the new console opens before others finish closing.
actions
Feedback: Rate this comment to help me improve future code reviews: