-
-
Notifications
You must be signed in to change notification settings - Fork 392
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 all commits
d48b37b
ef4b916
a05cfa7
7a13621
a2c9c2b
0c3b43c
683bbc3
b1a81a2
88f13ff
c61c752
2be7743
1f188ea
b231f2d
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 |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
using pyRevitLabs.Common; | ||
using pyRevitLabs.CommonWPF.Controls; | ||
using pyRevitLabs.Emojis; | ||
using pyRevitLabs.PyRevit; | ||
|
||
namespace PyRevitLabs.PyRevit.Runtime { | ||
public struct ScriptConsoleDebugger { | ||
|
@@ -171,7 +172,7 @@ public partial class ScriptConsole : ScriptConsoleTemplate, IComponentConnector, | |
private System.Windows.Forms.HtmlElement _lastDocumentBody = null; | ||
private UIApplication _uiApp; | ||
|
||
private List<ScriptConsoleDebugger> _supportedDebuggers = | ||
private List<ScriptConsoleDebugger> _supportedDebuggers = | ||
new List<ScriptConsoleDebugger> { | ||
new ScriptConsoleDebugger() { | ||
Name = "Pdb (IronPython|CPython)", | ||
|
@@ -376,6 +377,27 @@ public string GetFullHtml() { | |
return ScriptConsoleConfigs.DOCTYPE + head.OuterHtml + ActiveDocument.Body.OuterHtml; | ||
} | ||
|
||
private void ApplyCloseOthersConfig() | ||
{ | ||
if (PyRevitConfigs.GetCloseOtherOutputs()) | ||
{ | ||
var mode = PyRevitConfigs.GetCloseOutputMode(); | ||
this.Dispatcher.BeginInvoke(new Action(() => | ||
{ | ||
CloseOtherOutputs(filterByCommandId: mode == OutputCloseMode.CurrentCommand); | ||
})); | ||
} | ||
} | ||
|
||
public void CloseOtherOutputs(bool filterByCommandId = true) { | ||
try { | ||
var filterId = filterByCommandId ? this.OutputId : null; | ||
ScriptConsoleManager.CloseActiveOutputWindows(excludeOutputWindow: this, filterOutputWindowId: filterId); | ||
} | ||
catch { | ||
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. Empty catch block without logging. UI operations can fail for various reasons and silent failures make troubleshooting difficult. catch (Exception ex) {
// Log UI operation error for troubleshooting
System.Diagnostics.Debug.WriteLine($"Failed to close other outputs: {ex.Message}");
} 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. Empty catch block violates exception handling standards. According to the review validation checklist, empty catch blocks should log exceptions at minimum. Consider logging the exception or using more specific exception types. catch (Exception ex) {
logger.Debug("Failed to close other output windows: " + ex.Message);
} actionsFeedback: Rate this comment to help me improve future code reviews:
|
||
} | ||
} | ||
|
||
private void SetupDefaultPage(string styleSheetFilePath = null) { | ||
string cssFilePath; | ||
if (styleSheetFilePath != null) | ||
|
@@ -448,7 +470,7 @@ public void FocusOutput() { | |
|
||
public System.Windows.Forms.HtmlElement ComposeEntry(string contents, string HtmlElementType) { | ||
WaitReadyBrowser(); | ||
|
||
// order is important | ||
// "<" ---> < | ||
contents = ScriptConsoleConfigs.EscapeForHtml(contents); | ||
|
@@ -552,7 +574,7 @@ public string GetInput() { | |
dbgMode = true; | ||
} | ||
} | ||
|
||
// if no debugger, find other patterns | ||
if (!dbgMode && | ||
new string[] { "select", "file" }.All(x => lastLine.Contains(x))) | ||
|
@@ -782,6 +804,7 @@ public void SelfDestructTimer(int seconds) { | |
private void Window_Loaded(object sender, System.EventArgs e) { | ||
var outputWindow = (ScriptConsole)sender; | ||
ScriptConsoleManager.AppendToOutputWindowList(this); | ||
ApplyCloseOthersConfig(); | ||
} | ||
|
||
private void Window_Closing(object sender, System.ComponentModel.CancelEventArgs e) { | ||
|
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. 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:
|
||
|
||
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); | ||
} | ||
|
||
// cpythonengine | ||
public static int GetCpythonEngineVersion() | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,3 +38,5 @@ tooltip: | |
|
||
Zeigt die Konfigurationsdatei im Explorer an. | ||
context: zero-doc | ||
engine: | ||
clean: true |
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: