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

@@ -230,32 +230,11 @@ namespace SourceGen {
 
             private List<Tag> mSelectionTags = new List<Tag>();
 
-            /// <summary>
-            /// Holds data that helps us position the scroll view at the correct position
-            /// after changes have been applied.
-            /// </summary>
-            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 + "]";
-                }
-            }
-
             /// <summary>
             /// This is a place to save the file offset associated with the ListView's
             /// TopItem, so we can position the list appropriately.
             /// </summary>
-            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<Line> 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 {

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 };
         /// <summary>
         /// 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.
         /// </summary>
         /// <param name="gotoOffset">Offset to jump to.</param>
         /// <param name="doPush">If set, push new offset onto navigation stack.</param>
-        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");
+                if (doPush) {
-                return;
+                    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 {
             }
         }
 
+        /// <summary>
+        /// Calculates the currently-selected location.
+        /// </summary>
+        /// <remarks>
+        /// 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).
+        /// </remarks>
+        /// <returns>Returns the location.</returns>
         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);
         }
 
         /// <summary>
@@ -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");
                 }

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.
         /// </summary>
         public class Location {
+            /// <summary>
+            /// Code/data offset.  May be less than zero for equates.
+            /// </summary>
             public int Offset { get; set; }
+
+            /// <summary>
+            /// Number of lines between the first line at the specified offset, and the
+            /// line we actually want to land on.
+            /// </summary>
+            /// <remarks>
+            /// 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.
+            /// </remarks>
+            public int LineDelta { get; set; }
+
+            /// <summary>
+            /// True if the target is the note at the given offset, rather than the code/data.
+            /// </summary>
+            /// <remarks>
+            /// This is an alternative to LineDelta.
+            /// </remarks>
             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);
             }
         }
 

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();
         }