From 475c31b8867b875bb50619da8a8dac40f2f02d87 Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Thu, 10 Oct 2019 11:57:36 -0700 Subject: [PATCH] Tweak navigation If you select a local variable, double-click on a reference entry, and then hit "back", you aren't taken back to the correct place in the local variable table. This is annoying if you're trying to explore how a local variable is used. The NavStack Location object now has a "line delta" that can be applied to position the selection correctly. This isn't stable across undo/redo, but it solves the common cases. This makes LineListGen's "Top" class redundant, so uses of that have been replaced with Location. --- SourceGen/LineListGen.cs | 34 +++--------- SourceGen/MainController.cs | 84 ++++++++++++++++++++++------- SourceGen/NavStack.cs | 32 +++++++++-- SourceGen/WpfGui/MainWindow.xaml.cs | 6 ++- 4 files changed, 105 insertions(+), 51 deletions(-) diff --git a/SourceGen/LineListGen.cs b/SourceGen/LineListGen.cs index b195802..10e785f 100644 --- a/SourceGen/LineListGen.cs +++ b/SourceGen/LineListGen.cs @@ -230,32 +230,11 @@ namespace SourceGen { private List mSelectionTags = new List(); - /// - /// Holds data that helps us position the scroll view at the correct position - /// after changes have been applied. - /// - private class Top { - // File offset of line. - public int FileOffset { get; private set; } - // Number of lines between the first line at the specified offset and the - // target line. - public int LineDelta { get; private set; } - - public Top(int fileOffset, int lineDelta) { - FileOffset = fileOffset; - LineDelta = lineDelta; - Debug.WriteLine("New Top: " + this); - } - public override string ToString() { - return "[Top: off=+" + FileOffset.ToString("x6") + " delta=" + LineDelta + "]"; - } - } - /// /// This is a place to save the file offset associated with the ListView's /// TopItem, so we can position the list appropriately. /// - private Top mTopPosition; + private NavStack.Location mTopPosition; // Use Generate(). private SavedSelection() { } @@ -279,7 +258,8 @@ namespace SourceGen { int topOffset = dl[topIndex].FileOffset; int firstIndex = dl.FindLineIndexByOffset(topOffset); Debug.Assert(topIndex >= firstIndex); - savedSel.mTopPosition = new Top(topOffset, topIndex - firstIndex); + savedSel.mTopPosition = + new NavStack.Location(topOffset, topIndex - firstIndex, false); List lineList = dl.mLineList; Debug.Assert(lineList.Count == sel.Length); @@ -357,12 +337,14 @@ namespace SourceGen { // If a line encompassing this offset was at the top of the ListView // control before, use this line's index as the top. - if (topIndex < 0 && lineList[lineIndex].Contains(mTopPosition.FileOffset)) { + if (topIndex < 0 && lineList[lineIndex].Contains(mTopPosition.Offset)) { topIndex = lineIndex; } if (lineOffset >= tag.mOffset && lineOffset < tag.mOffset + tag.mSpan) { // Intersection. If the line type matches, add it to the set. + // TODO(someday): this is doing the wrong thing when we have more + // than one blank line at an offset. if ((tag.mTypes & lineList[lineIndex].LineType) != 0) { sel[lineIndex] = true; } @@ -380,7 +362,7 @@ namespace SourceGen { // Continue search for topIndex, if necessary. while (topIndex < 0 && lineIndex < lineList.Count) { - if (lineList[lineIndex].Contains(mTopPosition.FileOffset)) { + if (lineList[lineIndex].Contains(mTopPosition.Offset)) { topIndex = lineIndex; break; } @@ -394,7 +376,7 @@ namespace SourceGen { if (topIndex >= 0 && mTopPosition.LineDelta > 0) { int adjIndex = topIndex + mTopPosition.LineDelta; if (adjIndex >= lineList.Count || - lineList[adjIndex].FileOffset != mTopPosition.FileOffset) { + lineList[adjIndex].FileOffset != mTopPosition.Offset) { Debug.WriteLine("Can't adjust top position"); // can't adjust; maybe they deleted several lines from comment } else { diff --git a/SourceGen/MainController.cs b/SourceGen/MainController.cs index 25ca581..39d3807 100644 --- a/SourceGen/MainController.cs +++ b/SourceGen/MainController.cs @@ -1459,7 +1459,9 @@ namespace SourceGen { // (Resolve it as a numeric reference.) if (attr.OperandOffset >= 0) { // Yup, find the line for that offset and jump to it. - GoToOffset(attr.OperandOffset, false, true); + GoToLocation( + new NavStack.Location(attr.OperandOffset, 0, false), + GoToMode.JumpToCodeData, true); } else if (dfd != null && dfd.HasSymbol) { // Operand has a symbol, do a symbol lookup. if (dfd.SymbolRef.IsVariable) { @@ -1468,7 +1470,9 @@ namespace SourceGen { int labelOffset = mProject.FindLabelOffsetByName( dfd.SymbolRef.Label); if (labelOffset >= 0) { - GoToOffset(labelOffset, false, true); + GoToLocation( + new NavStack.Location(labelOffset, 0, false), + GoToMode.JumpToCodeData, true); } } } else if (attr.IsDataStart || attr.IsInlineDataStart) { @@ -1478,7 +1482,9 @@ namespace SourceGen { int operandOffset = DataAnalysis.GetDataOperandOffset( mProject, line.FileOffset); if (operandOffset >= 0) { - GoToOffset(operandOffset, false, true); + GoToLocation( + new NavStack.Location(operandOffset, 0, false), + GoToMode.JumpToCodeData, true); } } } @@ -2327,51 +2333,77 @@ namespace SourceGen { public void Goto() { GotoBox dlg = new GotoBox(mMainWin, mProject, mOutputFormatter); if (dlg.ShowDialog() == true) { - GoToOffset(dlg.TargetOffset, false, true); + GoToLocation(new NavStack.Location(dlg.TargetOffset, 0, false), + GoToMode.JumpToCodeData, true); mMainWin.CodeListView_Focus(); } } + public enum GoToMode { Unknown = 0, JumpToCodeData, JumpToNote, JumpToAdjIndex }; /// /// Moves the view and selection to the specified offset. We want to select stuff /// differently if we're jumping to a note vs. jumping to an instruction. /// /// Offset to jump to. /// If set, push new offset onto navigation stack. - public void GoToOffset(int gotoOffset, bool jumpToNote, bool doPush) { + public void GoToLocation(NavStack.Location loc, GoToMode mode, bool doPush) { NavStack.Location prevLoc = GetCurrentlySelectedLocation(); - if (gotoOffset == prevLoc.Offset && jumpToNote == prevLoc.IsNote) { + Debug.WriteLine("GoToLocation: " + loc + " mode=" + mode + " doPush=" + doPush + + " (curLoc=" + prevLoc + ")"); + + // Avoid pushing multiple copies of the same address on. This doesn't quite work + // because we can't compare the LineDelta without figuring out JumpToCodeData first. + // If we're sitting in a long comment or LvTable and the user double-clicks on the + // entry in the symbol table for the current offset, we want to move the selection, + // so we don't want to bail out if the offset matches. Easiest thing to do is to + // do the move but not push it. + bool jumpToNote = (mode == GoToMode.JumpToNote); + if (loc.Offset == prevLoc.Offset && jumpToNote == prevLoc.IsNote) { // we're jumping to ourselves? - Debug.WriteLine("Ignoring goto to current position"); - return; + if (doPush) { + Debug.WriteLine("Ignoring push for goto to current offset"); + doPush = false; + } } - int topLineIndex = CodeLineList.FindLineIndexByOffset(gotoOffset); + int topLineIndex = CodeLineList.FindLineIndexByOffset(loc.Offset); if (topLineIndex < 0) { - Debug.Assert(false, "failed goto offset +" + gotoOffset.ToString("x6")); + Debug.Assert(false, "failed goto offset +" + loc.Offset.ToString("x6")); return; } int lastLineIndex; - if (jumpToNote) { + if (mode == GoToMode.JumpToNote) { // Select all note lines, disregard the rest. while (CodeLineList[topLineIndex].LineType != LineListGen.Line.Type.Note) { topLineIndex++; - Debug.Assert(CodeLineList[topLineIndex].FileOffset == gotoOffset); + Debug.Assert(CodeLineList[topLineIndex].FileOffset == loc.Offset); } lastLineIndex = topLineIndex + 1; while (lastLineIndex < CodeLineList.Count && CodeLineList[lastLineIndex].LineType == LineListGen.Line.Type.Note) { lastLineIndex++; } - } else if (gotoOffset < 0) { + } else if (loc.Offset < 0) { // This is the offset of the header comment or a .EQ directive. Don't mess with it. lastLineIndex = topLineIndex + 1; - } else { + } else if (mode == GoToMode.JumpToCodeData) { // Advance to the code or data line. while (CodeLineList[topLineIndex].LineType != LineListGen.Line.Type.Code && CodeLineList[topLineIndex].LineType != LineListGen.Line.Type.Data) { topLineIndex++; } + + lastLineIndex = topLineIndex + 1; + } else if (mode == GoToMode.JumpToAdjIndex) { + // Adjust the line position by the line delta. If the adjustment moves us to + // a different element, ignore the adjustment. + if (CodeLineList[topLineIndex].FileOffset == + CodeLineList[topLineIndex + loc.LineDelta].FileOffset) { + topLineIndex += loc.LineDelta; + } + lastLineIndex = topLineIndex + 1; + } else { + Debug.Assert(false); lastLineIndex = topLineIndex + 1; } @@ -2450,6 +2482,18 @@ namespace SourceGen { } } + /// + /// Calculates the currently-selected location. + /// + /// + /// This is done whenever we jump somewhere else. For the most part we'll be in a + /// line of code, jumping when an operand or reference is double-clicked, but we might + /// be in the middle of a long comment when a symbol is double-clicked or the + /// nav-forward arrow is clicked. The most interesting case is when a reference for + /// a local variable table entry is double-clicked, since we want to be sure that we + /// return to the correct entry in the LvTable (assuming it still exists). + /// + /// Returns the location. private NavStack.Location GetCurrentlySelectedLocation() { int index = mMainWin.CodeListView_GetFirstSelectedIndex(); if (index < 0) { @@ -2457,8 +2501,9 @@ namespace SourceGen { index = mMainWin.CodeListView_GetTopIndex(); } int offset = CodeLineList[index].FileOffset; + int lineDelta = index - CodeLineList.FindLineIndexByOffset(offset); bool isNote = (CodeLineList[index].LineType == LineListGen.Line.Type.Note); - return new NavStack.Location(offset, isNote); + return new NavStack.Location(offset, lineDelta, isNote); } public bool CanNavigateBackward() { @@ -2467,7 +2512,8 @@ namespace SourceGen { public void NavigateBackward() { Debug.Assert(mNavStack.HasBackward); NavStack.Location backLoc = mNavStack.MoveBackward(GetCurrentlySelectedLocation()); - GoToOffset(backLoc.Offset, backLoc.IsNote, false); + GoToLocation(backLoc, + backLoc.IsNote ? GoToMode.JumpToNote : GoToMode.JumpToAdjIndex, false); } public bool CanNavigateForward() { @@ -2476,7 +2522,8 @@ namespace SourceGen { public void NavigateForward() { Debug.Assert(mNavStack.HasForward); NavStack.Location fwdLoc = mNavStack.MoveForward(GetCurrentlySelectedLocation()); - GoToOffset(fwdLoc.Offset, fwdLoc.IsNote, false); + GoToLocation(fwdLoc, + fwdLoc.IsNote ? GoToMode.JumpToNote : GoToMode.JumpToAdjIndex, false); } /// @@ -2487,7 +2534,8 @@ namespace SourceGen { if (sym.IsInternalLabel) { int offset = mProject.FindLabelOffsetByName(sym.Label); if (offset >= 0) { - GoToOffset(offset, false, true); + GoToLocation(new NavStack.Location(offset, 0, false), + GoToMode.JumpToCodeData, true); } else { Debug.WriteLine("DClick symbol: " + sym + ": label not found"); } diff --git a/SourceGen/NavStack.cs b/SourceGen/NavStack.cs index 43cdff8..a973ef2 100644 --- a/SourceGen/NavStack.cs +++ b/SourceGen/NavStack.cs @@ -31,7 +31,7 @@ namespace SourceGen { // previously-recorded forward places. // // Jumping to Notes is a little different from jumping to anything else, because we - // want to highlight the note rather than the code at the associated offset. This + // want to select the entire note rather than the code at the associated offset. This // is especially important when moving upward through the file, or the note will be // off the top of the screen. @@ -42,16 +42,38 @@ namespace SourceGen { /// Holds enough information to get us back where we were, in style. /// public class Location { + /// + /// Code/data offset. May be less than zero for equates. + /// public int Offset { get; set; } + + /// + /// Number of lines between the first line at the specified offset, and the + /// line we actually want to land on. + /// + /// + /// It's possible this line no longer exists. Easiest test is to compare the + /// offset of the unadjusted line to the offset of the adjusted line. If not + /// equal, ignore the delta. + /// + public int LineDelta { get; set; } + + /// + /// True if the target is the note at the given offset, rather than the code/data. + /// + /// + /// This is an alternative to LineDelta. + /// public bool IsNote { get; set; } - public Location(int offset, bool isNote) { + public Location(int offset, int lineDelta, bool isNote) { Offset = offset; + LineDelta = lineDelta; IsNote = isNote; } public override string ToString() { - return string.Format("[+{0:x6},{1}]", Offset, IsNote); + return string.Format("[+{0:x6},{1},{2}]", Offset, LineDelta, IsNote); } public static bool operator ==(Location a, Location b) { @@ -61,7 +83,7 @@ namespace SourceGen { if (ReferenceEquals(a, null) || ReferenceEquals(b, null)) { return false; // one is null } - return a.Offset == b.Offset && a.IsNote == b.IsNote; + return a.Offset == b.Offset && a.LineDelta == b.LineDelta && a.IsNote == b.IsNote; } public static bool operator !=(Location a, Location b) { return !(a == b); @@ -70,7 +92,7 @@ namespace SourceGen { return obj is Location && this == (Location)obj; } public override int GetHashCode() { - return Offset + (IsNote ? (1<<24) : 0); + return Offset + (LineDelta * 1000000) + (IsNote ? (1<<24) : 0); } } diff --git a/SourceGen/WpfGui/MainWindow.xaml.cs b/SourceGen/WpfGui/MainWindow.xaml.cs index 1aac2ed..5a5869c 100644 --- a/SourceGen/WpfGui/MainWindow.xaml.cs +++ b/SourceGen/WpfGui/MainWindow.xaml.cs @@ -1439,7 +1439,8 @@ namespace SourceGen.WpfGui { ReferencesListItem rli = (ReferencesListItem)item; // Jump to the offset, then shift the focus back to the code list. - mMainCtrl.GoToOffset(rli.OffsetValue, false, true); + mMainCtrl.GoToLocation(new NavStack.Location(rli.OffsetValue, 0, false), + MainController.GoToMode.JumpToCodeData, true); codeListView.Focus(); } @@ -1483,7 +1484,8 @@ namespace SourceGen.WpfGui { NotesListItem nli = (NotesListItem)item; // Jump to the offset, then shift the focus back to the code list. - mMainCtrl.GoToOffset(nli.OffsetValue, true, true); + mMainCtrl.GoToLocation(new NavStack.Location(nli.OffsetValue, 0, true), + MainController.GoToMode.JumpToNote, true); codeListView.Focus(); }