This is an automated email from the ASF dual-hosted git repository.
rstrickland pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/daffodil-vscode.git
The following commit(s) were added to refs/heads/main by this push:
new 25d9f1f Fix File/Viewport Navigation Issue
25d9f1f is described below
commit 25d9f1f37531f5491e53ca7ea3c867851dee58ed
Author: Davin Shearer <[email protected]>
AuthorDate: Thu Aug 17 13:44:58 2023 -0400
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 | 44 ++++++---
.../DataDisplays/CustomByteDisplay/BinaryData.ts | 2 -
.../CustomByteDisplay/DataLineFeed.svelte | 106 +++++++++++++--------
.../components/DataDisplays/DataViewports.svelte | 3 +-
src/svelte/src/components/dataEditor.svelte | 28 ++++--
src/svelte/src/stores/configuration.ts | 3 +
6 files changed, 120 insertions(+), 66 deletions(-)
diff --git a/src/dataEditor/dataEditorClient.ts
b/src/dataEditor/dataEditorClient.ts
index ae6bec1..467eeb6 100644
--- a/src/dataEditor/dataEditorClient.ts
+++ b/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) => {
diff --git
a/src/svelte/src/components/DataDisplays/CustomByteDisplay/BinaryData.ts
b/src/svelte/src/components/DataDisplays/CustomByteDisplay/BinaryData.ts
index e3f9e31..dec18d0 100644
--- a/src/svelte/src/components/DataDisplays/CustomByteDisplay/BinaryData.ts
+++ b/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'
diff --git
a/src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte
b/src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte
index f7d75cf..7051354 100644
---
a/src/svelte/src/components/DataDisplays/CustomByteDisplay/DataLineFeed.svelte
+++
b/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,8 +179,8 @@ 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
@@ -177,16 +188,20 @@ limitations under the License.
$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
@@ -194,7 +209,7 @@ limitations under the License.
}
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
diff --git a/src/svelte/src/components/DataDisplays/DataViewports.svelte
b/src/svelte/src/components/DataDisplays/DataViewports.svelte
index b031f25..2c0cafe 100644
--- a/src/svelte/src/components/DataDisplays/DataViewports.svelte
+++ b/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">
diff --git a/src/svelte/src/components/dataEditor.svelte
b/src/svelte/src/components/dataEditor.svelte
index 8af8309..a64acfe 100644
--- a/src/svelte/src/components/dataEditor.svelte
+++ b/src/svelte/src/components/dataEditor.svelte
@@ -46,7 +46,12 @@ limitations under the License.
import { vscode } from '../utilities/vscode'
import Header from './Header/Header.svelte'
import Main from './Main.svelte'
- import { EditByteModes, NUM_LINES_DISPLAYED } from '../stores/configuration'
+ import {
+ EditByteModes,
+ NUM_LINES_DISPLAYED,
+ VIEWPORT_CAPACITY_MAX,
+ VIEWPORT_SCROLL_INCREMENT,
+ } from '../stores/configuration'
import ServerMetrics from './ServerMetrics/ServerMetrics.svelte'
import {
elementKeypressEventMap,
@@ -86,7 +91,9 @@ limitations under the License.
const fileSize = $fileMetrics.computedSize
const viewportBoundary =
- $viewport.length + $viewport.fileOffset - 20 * $bytesPerRow
+ $viewport.length +
+ $viewport.fileOffset -
+ NUM_LINES_DISPLAYED * $bytesPerRow
const offset =
offsetArg > 0 &&
offsetArg < viewport.offsetMax &&
@@ -96,21 +103,26 @@ limitations under the License.
const relativeFileLine = Math.floor(offset / $bytesPerRow)
const relativeFileOffset = relativeFileLine * $bytesPerRow
- const lineTopBoundary = Math.floor($viewport.length / $bytesPerRow) - 20
+ const lineTopBoundary =
+ Math.floor($viewport.length / $bytesPerRow) - NUM_LINES_DISPLAYED
let relativeTargetLine = relativeFileLine
let viewportStartOffset = $viewport.fileOffset
-
+ $dataFeedAwaitRefresh = true
// make sure that the offset is within the loaded viewport
if (
offset < $viewport.fileOffset ||
offset > viewportBoundary ||
relativeTargetLine > lineTopBoundary
) {
- let adjustedFileOffset = Math.max(0, relativeFileOffset - 512)
- const fetchPastFileBoundary = fileSize - adjustedFileOffset < 1024
+ let adjustedFileOffset = Math.max(
+ 0,
+ relativeFileOffset - VIEWPORT_SCROLL_INCREMENT
+ )
+ const fetchPastFileBoundary =
+ fileSize - adjustedFileOffset < VIEWPORT_CAPACITY_MAX
if (fetchPastFileBoundary)
adjustedFileOffset = byte_count_divisible_offset(
- fileSize - 1024,
+ fileSize - VIEWPORT_CAPACITY_MAX,
$bytesPerRow,
1
)
@@ -124,7 +136,6 @@ limitations under the License.
) -
(NUM_LINES_DISPLAYED - 1)
: viewport_offset_to_line_num(offset, viewportStartOffset,
$bytesPerRow)
- $dataFeedAwaitRefresh = true
// NOTE: Scrolling the viewport will make the display bounce until it
goes to the correct offset
vscode.postMessage({
@@ -139,6 +150,7 @@ limitations under the License.
}
$dataFeedLineTop = relativeTargetLine
+ $dataFeedAwaitRefresh = false
clearDataDisplays()
}
diff --git a/src/svelte/src/stores/configuration.ts
b/src/svelte/src/stores/configuration.ts
index 8b681a1..37e3595 100644
--- a/src/svelte/src/stores/configuration.ts
+++ b/src/svelte/src/stores/configuration.ts
@@ -109,6 +109,9 @@ export const UNPRINTABLE_CHAR_STAND_IN =
String.fromCharCode(9617)
// Number of bytes to for the viewport to populate
export const VIEWPORT_CAPACITY_MAX = 16 * 64 // 1024, Ωedit maximum viewport
size is 1048576 (1024 * 1024)
+// Offset shift amount on viewport data fetch
+export const VIEWPORT_SCROLL_INCREMENT = VIEWPORT_CAPACITY_MAX / 2
+
// Number of bytes to display in the viewport
export const NUM_LINES_DISPLAYED = 20
