Skip to content

Commit

Permalink
fix tree hide/show bugs, reduce UI test crashes
Browse files Browse the repository at this point in the history 
1. Fix (long-standing) issue where closing a buffer that had no tree
    would not trigger re-opening of the tree for the buffer
    that Notepad++ automatically re-opened in response to the closure
2. Greatly reduce frequency of UI test crashes (to maybe 5% or less)
  • Loading branch information
@molsonkiko
molsonkiko committed Aug 15, 2023
1 parent 0537df4 commit 962a7df
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 131 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

1. Eliminated some annoying but harmless plugin crashes.
2. Fix long-standing issue where closing buffer `A` with no associated treeview would not re-open the treeview of the file `B` automatically opened by Notepad++ when `A` was closed.
3. Dramatically reduce the frequency of Notepad++ freezing and crashing while running UI tests.

## [5.5.0] - 2023-08-13

Expand Down
247 changes: 123 additions & 124 deletions JsonToolsNppPlugin/Main.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class Main
// sometimes it is best to forget selections if the user performs an action
// that makes the selections no longer map to the JSON elements that had been selected
// warn the user the first time this happens
static bool hasWarnedSelectionsForgotten = false;
public static bool hasWarnedSelectionsForgotten = false;
// sort form stuff
public static SortForm sortForm = null;
// error form stuff
Expand Down Expand Up @@ -144,55 +144,35 @@ public static void OnNotification(ScNotification notification)
//// changing tabs
switch (code)
{
case (uint)NppMsg.NPPN_FILEBEFOREOPEN:
bufferFinishedOpening = false;
break;
case (uint)NppMsg.NPPN_BUFFERACTIVATED:
bufferFinishedOpening = true;
IsCurrentFileBig();
// When a new buffer is activated, we need to reset the connector to the Scintilla editing component.
// This is usually unnecessary, but if there are multiple instances or multiple views,
// we need to track which of the currently visible buffers are actually being edited.
Npp.editor = new ScintillaGateway(PluginBase.GetCurrentScintilla());
string newFname = Npp.notepad.GetFilePath(notification.Header.IdFrom);
JsonFileInfo info = null;
JsonFileInfo newInfo = null;
bool lastFileHasInfo = activeFname != null && TryGetInfoForFile(activeFname, out info);
bool newFileHasInfo = TryGetInfoForFile(newFname, out newInfo);
if (lastFileHasInfo)
case (uint)NppMsg.NPPN_FILEBEFOREOPEN:
bufferFinishedOpening = false;
break;
case (uint)NppMsg.NPPN_BUFFERACTIVATED:
bufferFinishedOpening = true;
IsCurrentFileBig();
// When a new buffer is activated, we need to reset the connector to the Scintilla editing component.
// This is usually unnecessary, but if there are multiple instances or multiple views,
// we need to track which of the currently visible buffers are actually being edited.
Npp.editor = new ScintillaGateway(PluginBase.GetCurrentScintilla());
string newFname = Npp.notepad.GetFilePath(notification.Header.IdFrom);
JsonFileInfo info = null;
JsonFileInfo newInfo = null;
bool lastFileHasInfo = activeFname != null && TryGetInfoForFile(activeFname, out info);
bool newFileHasInfo = TryGetInfoForFile(newFname, out newInfo);
if (lastFileHasInfo)
{
// check to see if there was a treeview for the file
// we just navigated away from
if (openTreeViewer != null && openTreeViewer.IsDisposed)
{
// check to see if there was a treeview for the file
// we just navigated away from
if (openTreeViewer != null && openTreeViewer.IsDisposed)
{
info.tv = null;
jsonFileInfos[activeFname] = info;
openTreeViewer = null;
}
// now see if there's a treeview for the file just opened

if (newFileHasInfo
&& newInfo.tv != null)
{
if (newInfo.tv.IsDisposed)
{
newInfo.tv = null;
jsonFileInfos[newFname] = newInfo;
}
else
{
// minimize the treeviewer that was visible and make the new one visible
if (openTreeViewer != null)
Npp.notepad.HideDockingForm(openTreeViewer);
Npp.notepad.ShowDockingForm(newInfo.tv);
openTreeViewer = newInfo.tv;
}
}
// else { }
// don't hide the active TreeViewer just because the user opened a new tab,
// they might want to still have it open
info.tv = null;
jsonFileInfos[activeFname] = info;
openTreeViewer = null;
}
if (newFileHasInfo && newInfo.usesSelections && newInfo.json != null && newInfo.json is JObject newObj && newObj.Length > 0)
}
if (newFileHasInfo)
{
if (newInfo.usesSelections && newInfo.json != null && newInfo.json is JObject newObj && newObj.Length > 0)
{
// Notepad++ doesn't remember all selections if there are multiple; it only remembers one.
// This poses problems for our selection remembering
Expand All @@ -203,94 +183,113 @@ public static void OnNotification(ScNotification notification)
int firstSelStart = newObj.children.Keys.Min(SelectionManager.StartFromStartEnd);
SelectionManager.SetSelectionsFromStartEnds(new string[] { $"{firstSelStart},{firstSelStart}" });
}
// if grepper form and tree viewer are both open,
// ensure that only one is visible at a time
// don't do anything when the tree view first opens though
if (grepperForm != null
&& grepperForm.tv != null
&& !grepperForm.tv.IsDisposed
&& !grepperTreeViewJustOpened)
if (newInfo.tv != null)
{
if (Npp.notepad.GetCurrentFilePath() == grepperForm.tv.fname)
if (newInfo.tv.IsDisposed)
{
if (openTreeViewer != null && !openTreeViewer.IsDisposed)
Npp.notepad.HideDockingForm(openTreeViewer);
Npp.notepad.ShowDockingForm(grepperForm.tv);
newInfo.tv = null;
jsonFileInfos[newFname] = newInfo;
}
else
{
Npp.notepad.HideDockingForm(grepperForm.tv);
if (openTreeViewer != null && !openTreeViewer.IsDisposed)
Npp.notepad.ShowDockingForm(openTreeViewer);
// minimize the treeviewer that was visible (if any) and make the new one visible
if (openTreeViewer != null)
Npp.notepad.HideDockingForm(openTreeViewer);
Npp.notepad.ShowDockingForm(newInfo.tv);
openTreeViewer = newInfo.tv;
}
}
grepperTreeViewJustOpened = false;
activeFname = newFname;
if (!settings.auto_validate) // if auto_validate is turned on, it'll be validated anyway
ValidateIfFilenameMatches(newFname);
if (newFileHasInfo && newInfo.statusBarSection != null)
Npp.notepad.SetStatusBarSection(newInfo.statusBarSection, StatusBarSection.DocType);
return;
// when closing a file
case (uint)NppMsg.NPPN_FILEBEFORECLOSE:
IntPtr bufferClosedId = notification.Header.IdFrom;
string bufferClosed = Npp.notepad.GetFilePath(bufferClosedId);
if (TryGetInfoForFile(bufferClosed, out JsonFileInfo closedInfo))
}
// if grepper form and tree viewer are both open,
// ensure that only one is visible at a time
// don't do anything when the tree view first opens though
if (grepperForm != null
&& grepperForm.tv != null
&& !grepperForm.tv.IsDisposed
&& !grepperTreeViewJustOpened)
{
if (Npp.notepad.GetCurrentFilePath() == grepperForm.tv.fname)
{
closedInfo.Dispose();
jsonFileInfos.Remove(bufferClosed);
if (openTreeViewer != null && !openTreeViewer.IsDisposed)
Npp.notepad.HideDockingForm(openTreeViewer);
Npp.notepad.ShowDockingForm(grepperForm.tv);
}
// if you close the file belonging the GrepperForm, delete its tree viewer
if (grepperForm != null && grepperForm.tv != null
&& !grepperForm.tv.IsDisposed
&& bufferClosed == grepperForm.tv.fname)
else
{
Npp.notepad.HideDockingForm(grepperForm.tv);
grepperForm.tv.Close();
grepperForm.tv = null;
return;
if (openTreeViewer != null && !openTreeViewer.IsDisposed)
Npp.notepad.ShowDockingForm(openTreeViewer);
}
}
grepperTreeViewJustOpened = false;
activeFname = newFname;
if (!settings.auto_validate) // if auto_validate is turned on, it'll be validated anyway
ValidateIfFilenameMatches(newFname);
if (newFileHasInfo && newInfo.statusBarSection != null)
Npp.notepad.SetStatusBarSection(newInfo.statusBarSection, StatusBarSection.DocType);
return;
// when closing a file
case (uint)NppMsg.NPPN_FILEBEFORECLOSE:
IntPtr bufferClosedId = notification.Header.IdFrom;
string bufferClosed = Npp.notepad.GetFilePath(bufferClosedId);
if (TryGetInfoForFile(bufferClosed, out JsonFileInfo closedInfo))
{
if (openTreeViewer != null && openTreeViewer == closedInfo.tv)
openTreeViewer = null;
closedInfo.Dispose();
jsonFileInfos.Remove(bufferClosed);
}
// if you close the file belonging the GrepperForm, delete its tree viewer
if (grepperForm != null && grepperForm.tv != null
&& !grepperForm.tv.IsDisposed
&& bufferClosed == grepperForm.tv.fname)
{
Npp.notepad.HideDockingForm(grepperForm.tv);
grepperForm.tv.Close();
grepperForm.tv = null;
return;
// the editor color scheme changed, so update form colors
case (uint)NppMsg.NPPN_WORDSTYLESUPDATED:
RestyleEverything();
return;
// Before a file is renamed or saved, add a note of the
// buffer id of the associated treeviewer and what its old name was.
// That way, the treeviewer can be renamed later.
// If you do nothing, the renamed treeviewers will be unreachable and
// the plugin will crash when Notepad++ closes.
case (uint)NppMsg.NPPN_FILEBEFORESAVE:
case (uint)NppMsg.NPPN_FILEBEFORERENAME:
FileBeforeRename(notification.Header.IdFrom);
return;
// After the file is renamed or saved:
// 1. change the fname attribute of any treeViewers that were renamed.
// 2. Remap the new fname to that treeviewer and remove the old fname from treeViewers.
// 3. when the schemas to fname patterns file is saved, parse it and validate to make sure it's ok
// 4. if the file matches a pattern in schemasToFnamePatterns, validate with the appropriate schema
case (uint)NppMsg.NPPN_FILESAVED:
case (uint)NppMsg.NPPN_FILERENAMED:
FileRenamed(notification.Header.IdFrom);
return;
// if a treeviewer was slated for renaming, just cancel that
case (uint)NppMsg.NPPN_FILERENAMECANCEL:
FileRenameCancel(notification.Header.IdFrom);
return;
// if the user did nothing for a while (default 1 second) after editing,
// re-parse the file and also perform validation if that's enabled.
case (uint)SciMsg.SCN_MODIFIED:
// only turn on the flag if the user performed the modification
lastEditedTime = System.DateTime.UtcNow;
ChangeJsonSelectionsBasedOnEdit(notification);
if (openTreeViewer != null)
openTreeViewer.shouldRefresh = true;
break;
//if (code > int.MaxValue) // windows messages
//{
// int wm = -(int)code;
// }
//}
}
return;
// the editor color scheme changed, so update form colors
case (uint)NppMsg.NPPN_WORDSTYLESUPDATED:
RestyleEverything();
return;
// Before a file is renamed or saved, add a note of the
// buffer id of the associated treeviewer and what its old name was.
// That way, the treeviewer can be renamed later.
// If you do nothing, the renamed treeviewers will be unreachable and
// the plugin will crash when Notepad++ closes.
case (uint)NppMsg.NPPN_FILEBEFORESAVE:
case (uint)NppMsg.NPPN_FILEBEFORERENAME:
FileBeforeRename(notification.Header.IdFrom);
return;
// After the file is renamed or saved:
// 1. change the fname attribute of any treeViewers that were renamed.
// 2. Remap the new fname to that treeviewer and remove the old fname from treeViewers.
// 3. when the schemas to fname patterns file is saved, parse it and validate to make sure it's ok
// 4. if the file matches a pattern in schemasToFnamePatterns, validate with the appropriate schema
case (uint)NppMsg.NPPN_FILESAVED:
case (uint)NppMsg.NPPN_FILERENAMED:
FileRenamed(notification.Header.IdFrom);
return;
// if a treeviewer was slated for renaming, just cancel that
case (uint)NppMsg.NPPN_FILERENAMECANCEL:
FileRenameCancel(notification.Header.IdFrom);
return;
// if the user did nothing for a while (default 1 second) after editing,
// re-parse the file and also perform validation if that's enabled.
case (uint)SciMsg.SCN_MODIFIED:
// only turn on the flag if the user performed the modification
lastEditedTime = System.DateTime.UtcNow;
ChangeJsonSelectionsBasedOnEdit(notification);
if (openTreeViewer != null)
openTreeViewer.shouldRefresh = true;
break;
//if (code > int.MaxValue) // windows messages
//{
// int wm = -(int)code;
// }
//}
}
}

Expand Down Expand Up @@ -1540,7 +1539,7 @@ static bool ValidateIfFilenameMatches(string fname, bool was_autotriggered = fal
/// </summary>
public static void OpenSortForm()
{
if (sortForm == null)
if (sortForm == null || sortForm.IsDisposed)
sortForm = new SortForm();
sortForm.PathTextBox.Text = PathToPosition(KeyStyle.RemesPath);
sortForm.Show();
Expand Down
4 changes: 2 additions & 2 deletions JsonToolsNppPlugin/Properties/AssemblyInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,5 @@
// Build Number
// Revision
//
[assembly: AssemblyVersion("5.5.0.1")]
[assembly: AssemblyFileVersion("5.5.0.1")]
[assembly: AssemblyVersion("5.5.0.2")]
[assembly: AssemblyFileVersion("5.5.0.2")]
16 changes: 11 additions & 5 deletions JsonToolsNppPlugin/Tests/UserInterfaceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,17 @@ public static bool Test()
int previousIndentPrettyPrint = Main.settings.indent_pretty_print;
bool previousMinimalWhiteSpaceCompression = Main.settings.minimal_whitespace_compression;
int previousMaxTrackedJsonSelections = Main.settings.max_tracked_json_selections;
bool previousHasWarnedSelectionsForgotten = Main.hasWarnedSelectionsForgotten;
// require these settings for the UI tests alone
Main.settings.pretty_print_style = PrettyPrintStyle.PPrint;
Main.settings.tab_indent_pretty_print = false;
Main.settings.indent_pretty_print = 4;
Main.settings.minimal_whitespace_compression = true;
Main.settings.max_tracked_json_selections = 1000;
// if this is false, a message-box will pop up at some point.
// this message box doesn't block the main thread, but it introduces some asynchronous behavior
// that was probably responsible for crashing the UI tests
Main.hasWarnedSelectionsForgotten = true;
// add command to overwrite with a lot of arrays and select every valid json
try
{
Expand Down Expand Up @@ -394,11 +399,6 @@ public static bool Test()
lastFailureIndex = messages.Count;
}
}
Main.settings.pretty_print_style = previousPrettyPrintStyle;
Main.settings.indent_pretty_print = previousIndentPrettyPrint;
Main.settings.tab_indent_pretty_print = previousTabIndentPrettyPrint;
Main.settings.minimal_whitespace_compression = previousMinimalWhiteSpaceCompression;
Main.settings.max_tracked_json_selections = previousMaxTrackedJsonSelections;
// go back to the test file and show the results
Npp.notepad.OpenFile(previouslyOpenFname);
if (failures > 0)
Expand All @@ -408,6 +408,12 @@ public static bool Test()
}
Npp.AddLine($"Failed {failures} tests");
Npp.AddLine($"Passed {testcases.Count - failures} tests");
Main.settings.pretty_print_style = previousPrettyPrintStyle;
Main.settings.indent_pretty_print = previousIndentPrettyPrint;
Main.settings.tab_indent_pretty_print = previousTabIndentPrettyPrint;
Main.settings.minimal_whitespace_compression = previousMinimalWhiteSpaceCompression;
Main.settings.max_tracked_json_selections = previousMaxTrackedJsonSelections;
Main.hasWarnedSelectionsForgotten = previousHasWarnedSelectionsForgotten;
return failures > 0;
}
}
Expand Down

0 comments on commit 962a7df

Please sign in to comment.