Fix File/Viewport Navigation Issue

- Fixes issue where the UI would send viewport data requests w/ invalid
  offsets if the filesize was less than the viewport capacity.
- Reworked `getLogger` functionality of data editor extension.

Closes #819

src/dataEditor/dataEditorClient.ts

@@ -245,9 +245,14 @@ export class DataEditorClient implements vscode.Disposable {
         ? (createSessionResponse.getFileSize() as number)
         : 0
     } catch {
-      vscode.window.showErrorMessage(
-        `Failed to create session for ${this.fileToEdit}`
-      )
+      const msg = `Failed to create session for ${this.fileToEdit}`
+      getLogger().error({
+        err: {
+          msg: msg,
+          stack: new Error().stack,
+        },
+      })
+      vscode.window.showErrorMessage(msg)
     }
 
     // create the viewport
@@ -264,9 +269,14 @@ export class DataEditorClient implements vscode.Disposable {
       await viewportSubscribe(this.panel, this.currentViewportId)
       await sendViewportRefresh(this.panel, viewportDataResponse)
     } catch {
-      vscode.window.showErrorMessage(
-        `Failed to create viewport for ${this.fileToEdit}`
-      )
+      const msg = `Failed to create viewport for ${this.fileToEdit}`
+      getLogger().error({
+        err: {
+          msg: msg,
+          stack: new Error().stack,
+        },
+      })
+      vscode.window.showErrorMessage(msg)
     }
 
     // send the initial file info to the webview
@@ -602,17 +612,22 @@ export class DataEditorClient implements vscode.Disposable {
     offset: number,
     bytesPerRow: number
   ) {
-    // start of the row containing the offset
-    const startOffset = offset - (offset % bytesPerRow)
+    // start of the row containing the offset, making sure the offset is never negative
+    const startOffset = Math.max(0, offset - (offset % bytesPerRow))
     try {
       await sendViewportRefresh(
         panel,
         await modifyViewport(viewportId, startOffset, VIEWPORT_CAPACITY_MAX)
       )
     } catch {
-      vscode.window.showErrorMessage(
-        `Failed to scroll viewport ${viewportId} to offset ${startOffset}`
-      )
+      const msg = `Failed to scroll viewport ${viewportId} to offset ${startOffset}`
+      getLogger().error({
+        err: {
+          msg: msg,
+          stack: new Error().stack,
+        },
+      })
+      vscode.window.showErrorMessage(msg)
     }
   }
 }
@@ -816,9 +831,10 @@ async function viewportSubscribe(
         .setInterest(ALL_EVENTS & ~ViewportEventKind.VIEWPORT_EVT_MODIFY)
     )
     .on('data', async (event: ViewportEvent) => {
-      getLogger().debug(
-        `viewport '${event.getViewportId()}' received event: ${event.getViewportEventKind()}`
-      )
+      getLogger().debug({
+        viewportId: event.getViewportId(),
+        event: event.getViewportEventKind(),
+      })
       await sendViewportRefresh(panel, await getViewportData(viewportId))
     })
     .on('error', (err) => {

src/svelte/src/components/DataDisplays/CustomByteDisplay/BinaryData.ts

@@ -21,8 +21,6 @@ import { radixBytePad } from '../../../utilities/display'
 
 export const BYTE_ACTION_DIV_OFFSET: number = 24
 
-export const VIEWPORT_SCROLL_INCREMENT: number = 512
-
 export type EditAction =
   | 'insert-before'
   | 'insert-after'

src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte

@@ -21,21 +21,21 @@ limitations under the License.
     editMode,
     editorEncoding,
     focusedViewportId,
-    seekOffsetInput,
     selectionDataStore,
     selectionSize,
     selectedByte,
     fileMetrics,
     searchQuery,
     editorActionsAllowed,
+    dataFeedLineTop,
   } from '../../../stores'
   import {
     EditByteModes,
     NUM_LINES_DISPLAYED,
-    VIEWPORT_CAPACITY_MAX,
     type BytesPerRow,
     type RadixValues,
     EditActionRestrictions,
+    VIEWPORT_SCROLL_INCREMENT,
   } from '../../../stores/configuration'
   import { MessageCommand } from '../../../utilities/message'
   import { vscode } from '../../../utilities/vscode'
@@ -65,8 +65,8 @@ limitations under the License.
     updateSearchResultsHighlights,
   } from '../../../utilities/highlights'
 
-  export let lineTop: number
-  export let awaitViewportScroll: boolean
+  // export let $dataFeedLineTop: number
+  export let awaitViewportSeek: boolean
   export let bytesPerRow: BytesPerRow = 16
   export let dataRadix: RadixValues = 16
   export let addressRadix: RadixValues = 16
@@ -76,43 +76,53 @@ limitations under the License.
   const CONTAINER_ID = 'viewportData-container'
   const eventDispatcher = createEventDispatcher()
 
-  function OFFSET_FETCH_ADJUSTMENT(direction: ViewportScrollDirection) {
+  function OFFSET_FETCH_ADJUSTMENT(
+    direction: ViewportScrollDirection,
+    numLinesToScroll: number
+  ) {
+    const newLineTopOffset =
+      numLinesToScroll * bytesPerRow + $dataFeedLineTop * bytesPerRow
+    let scroll_count = Math.floor(newLineTopOffset / VIEWPORT_SCROLL_INCREMENT)
+
     if (direction === ViewportScrollDirection.INCREMENT) {
-      const fetchBound = viewportData.fileOffset + VIEWPORT_CAPACITY_MAX / 2
+      const fetchBound =
+        viewportData.fileOffset + scroll_count * VIEWPORT_SCROLL_INCREMENT
       if (fetchBound > $fileMetrics.computedSize)
         return (
-          (fetchBound - $fileMetrics.computedSize / bytesPerRow) * bytesPerRow
+          ($fileMetrics.computedSize / bytesPerRow) * bytesPerRow -
+          NUM_LINES_DISPLAYED * bytesPerRow
         )
+
       return fetchBound
     } else {
       const validBytesRemaining =
-        viewportData.fileOffset - VIEWPORT_CAPACITY_MAX / 2 > 0
+        viewportData.fileOffset + scroll_count * VIEWPORT_SCROLL_INCREMENT > 0
       if (!validBytesRemaining) return 0
       else {
-        return viewportData.fileOffset - VIEWPORT_CAPACITY_MAX / 2
+        return (
+          viewportData.fileOffset + scroll_count * VIEWPORT_SCROLL_INCREMENT
+        )
       }
     }
   }
 
   const INCREMENT_LINE = () => {
-    handle_navigation(ViewportScrollDirection.INCREMENT)
+    handle_navigation(1)
   }
   const DECREMENT_LINE = () => {
-    handle_navigation(ViewportScrollDirection.DECREMENT)
+    handle_navigation(-1)
   }
   const INCREMENT_SEGMENT = () => {
-    handle_navigation(ViewportScrollDirection.INCREMENT, NUM_LINES_DISPLAYED)
+    handle_navigation(NUM_LINES_DISPLAYED)
   }
   const DECREMENT_SEGMENT = () => {
-    handle_navigation(ViewportScrollDirection.DECREMENT, -NUM_LINES_DISPLAYED)
+    handle_navigation(-NUM_LINES_DISPLAYED)
   }
   const SCROLL_TO_END = () => {
-    $seekOffsetInput = $fileMetrics.computedSize.toString(addressRadix)
-    eventDispatcher('seek')
+    handle_navigation(lineTopMaxFile)
   }
   const SCROLL_TO_TOP = () => {
-    $seekOffsetInput = '0'
-    eventDispatcher('seek')
+    handle_navigation(-lineTopMaxFile)
   }
 
   let totalLinesPerFilesize = 0
@@ -139,6 +149,7 @@ limitations under the License.
 
   enum ViewportScrollDirection {
     DECREMENT = -1,
+    NONE = 0,
     INCREMENT = 1,
   }
 
@@ -168,33 +179,37 @@ limitations under the License.
       0
     )
 
-    atViewportHead = lineTop === 0
-    atViewportTail = lineTop === lineTopMaxViewport
+    atViewportHead = $dataFeedLineTop === 0
+    atViewportTail = $dataFeedLineTop === lineTopMaxViewport
     atFileHead = viewportData.fileOffset === 0
     atFileTail = viewportData.bytesLeft === 0
 
     disableDecrement =
       $selectionDataStore.active || (atViewportHead && atFileHead)
     disableIncrement =
       $selectionDataStore.active || (atViewportTail && atFileTail)
-    lineTopFileOffset = lineTop * bytesPerRow
+    lineTopFileOffset = $dataFeedLineTop * bytesPerRow
   }
 
   $: {
-    if (viewportData.fileOffset >= 0 && !awaitViewportScroll && lineTop >= 0) {
+    if (
+      viewportData.fileOffset >= 0 &&
+      !awaitViewportSeek &&
+      $dataFeedLineTop >= 0
+    ) {
       if (
         viewportLines.length !== 0 &&
         bytesPerRow !== viewportLines[0].bytes.length
       ) {
-        lineTop = viewport_offset_to_line_num(
+        $dataFeedLineTop = viewport_offset_to_line_num(
           parseInt(viewportLines[0].offset, addressRadix),
           viewportData.fileOffset,
           bytesPerRow
         )
       }
 
       viewportLines = generate_line_data(
-        lineTop,
+        $dataFeedLineTop,
         dataRadix,
         addressRadix,
         bytesPerRow
@@ -273,29 +288,37 @@ limitations under the License.
       : atViewportTail && atFileTail
   }
 
-  function handle_navigation(
-    direction: ViewportScrollDirection,
-    linesToMove: number = direction
-  ) {
-    if (at_scroll_boundary(direction)) return
+  function direction_of_scroll(
+    numLinesToScroll: number
+  ): ViewportScrollDirection {
+    return Math.sign(numLinesToScroll) as ViewportScrollDirection
+  }
+
+  function handle_navigation(numLinesToScroll: number) {
+    const navDirection = direction_of_scroll(numLinesToScroll)
 
-    if (at_fetch_boundary(direction, linesToMove)) {
+    if (at_scroll_boundary(navDirection)) return
+
+    if (at_fetch_boundary(navDirection, numLinesToScroll)) {
       const viewportOffset = viewportData.fileOffset
       const lineTopOffset = viewportLines[0].bytes[0].offset
-      const nextViewportOffset = OFFSET_FETCH_ADJUSTMENT(direction)
+      const nextViewportOffset = OFFSET_FETCH_ADJUSTMENT(
+        navDirection,
+        numLinesToScroll
+      )
 
       eventDispatcher('traverse-file', {
         nextViewportOffset: nextViewportOffset,
         lineTopOnRefresh:
           Math.floor(
             (viewportOffset + lineTopOffset - nextViewportOffset) / bytesPerRow
-          ) + linesToMove,
+          ) + numLinesToScroll,
       })
       return
     }
 
-    const newLine = lineTop + linesToMove
-    lineTop = Math.max(0, Math.min(newLine, lineTopMaxViewport))
+    const newLine = $dataFeedLineTop + numLinesToScroll
+    $dataFeedLineTop = Math.max(0, Math.min(newLine, lineTopMaxViewport))
   }
 
   function at_fetch_boundary(
@@ -304,8 +327,8 @@ limitations under the License.
   ): boolean {
     if (linesToMove != direction)
       return direction === ViewportScrollDirection.INCREMENT
-        ? lineTop + linesToMove >= lineTopMaxViewport && !atFileTail
-        : lineTop + linesToMove <= 0 && !atFileHead
+        ? $dataFeedLineTop + linesToMove >= lineTopMaxViewport && !atFileTail
+        : $dataFeedLineTop + linesToMove <= 0 && !atFileHead
 
     return direction === ViewportScrollDirection.INCREMENT
       ? atViewportTail && !atFileTail
@@ -410,7 +433,7 @@ limitations under the License.
         },
       })
       lineTopOnRefresh = lineTopMaxViewport
-      awaitViewportScroll = true
+      awaitViewportSeek = true
     }
   }
 
@@ -433,9 +456,12 @@ limitations under the License.
   window.addEventListener('message', (msg) => {
     switch (msg.data.command) {
       case MessageCommand.viewportRefresh:
-        if (awaitViewportScroll) {
-          awaitViewportScroll = false
-          lineTop = Math.max(0, Math.min(lineTopMaxViewport, lineTop))
+        if (awaitViewportSeek) {
+          awaitViewportSeek = false
+          $dataFeedLineTop = Math.max(
+            0,
+            Math.min(lineTopMaxViewport, $dataFeedLineTop)
+          )
           if ($selectionDataStore.active)
             selectedByteElement = document.getElementById(
               $selectedByte.offset.toString()
@@ -520,7 +546,7 @@ limitations under the License.
     <FileTraversalIndicator
       totalLines={totalLinesPerFilesize}
       selectionActive={$selectionDataStore.active}
-      currentLine={lineTop}
+      currentLine={$dataFeedLineTop}
       fileOffset={viewportData.fileOffset}
       maxDisplayLines={NUM_LINES_DISPLAYED}
       bind:percentageTraversed

src/svelte/src/components/DataDisplays/DataViewports.svelte

@@ -32,11 +32,10 @@ limitations under the License.
   on:applyChanges
   on:handleEditorEvent
   viewportData={$viewport}
-  lineTop={$dataFeedLineTop}
   bytesPerRow={$bytesPerRow}
   dataRadix={$displayRadix}
   addressRadix={$addressRadix}
-  bind:awaitViewportScroll={$dataFeedAwaitRefresh}
+  bind:awaitViewportSeek={$dataFeedAwaitRefresh}
 />
 
 <style lang="scss">