-
Notifications
You must be signed in to change notification settings - Fork 19
feat(URL): Support seeking logs by timestamp via URL parameter (resolves #117). #152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 45 commits
5477d0a
4a8aeb7
7f5cc7f
54ef03c
6c2b45a
255f3e3
3972319
5ecdaab
f1f93f7
683350a
7b72d93
0cc3649
a81d570
e8e58f5
206b01f
07ef527
6c910d4
bc16de2
e04dadb
3c051e8
32de344
127d56b
e5725c2
56c5a1f
3fa1b87
2074a5d
ca539cf
6e1c1dd
2ba9e82
d2c2a62
dc21b01
012a66c
67ff878
7b53dab
251f602
535172f
470db18
7114402
61241bc
85d563c
4782586
e458a4c
f3b9492
b99b23a
95fbb0c
46124e1
8506148
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,32 +3,149 @@ import React, { | |
| useRef, | ||
| } from "react"; | ||
|
|
||
| import useLogFileManagerProxyStore from "../stores/logFileManagerProxyStore"; | ||
| import useLogFileStore from "../stores/logFileStore"; | ||
| import {handleErrorWithNotification} from "../stores/notificationStore"; | ||
| import useQueryStore from "../stores/queryStore"; | ||
| import useViewStore from "../stores/viewStore"; | ||
| import {Nullable} from "../typings/common"; | ||
| import { | ||
| CURSOR_CODE, | ||
| CursorType, | ||
| } from "../typings/worker"; | ||
| import { | ||
| findNearestLessThanOrEqualElement, | ||
| isWithinBounds, | ||
| } from "../utils/data"; | ||
| import {clamp} from "../utils/math.ts"; | ||
| import { | ||
| getWindowUrlHashParams, | ||
| getWindowUrlSearchParams, | ||
| updateWindowUrlHashParams, | ||
| URL_HASH_PARAMS_DEFAULT, | ||
| URL_SEARCH_PARAMS_DEFAULT, | ||
| } from "../utils/url"; | ||
|
|
||
|
|
||
| /** | ||
| * Updates the log event number in the URL to `logEventNum` if it's within the bounds of | ||
| * `logEventNumsOnPage`. | ||
| * | ||
| * @param logEventNum | ||
| * @param logEventNumsOnPage | ||
| * @return Whether `logEventNum` is within the bounds of `logEventNumsOnPage`. | ||
| */ | ||
| const updateUrlIfEventOnPage = ( | ||
| logEventNum: number, | ||
| logEventNumsOnPage: number[] | ||
| ): boolean => { | ||
| if (false === isWithinBounds(logEventNumsOnPage, logEventNum)) { | ||
Henry8192 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
Henry8192 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| const nearestIdx = findNearestLessThanOrEqualElement( | ||
| logEventNumsOnPage, | ||
| logEventNum | ||
| ); | ||
|
|
||
| // Since `isWithinBounds` returned `true`, then: | ||
| // - `logEventNumsOnPage` must bound `logEventNum`. | ||
| // - `logEventNumsOnPage` cannot be empty. | ||
| // - `nearestIdx` cannot be `null`. | ||
| // | ||
| // Therefore, we can safely cast: | ||
| // - `nearestIdx` from `Nullable<number>` to `number`. | ||
| // - `logEventNumsOnPage[nearestIdx]` from `number | undefined` to `number`. | ||
| const nearestLogEventNum = logEventNumsOnPage[nearestIdx as number] as number; | ||
|
|
||
| updateWindowUrlHashParams({ | ||
| logEventNum: nearestLogEventNum, | ||
| }); | ||
|
|
||
| return true; | ||
| }; | ||
|
|
||
|
|
||
| /** | ||
| * Determines the cursor for navigating log events based on URL hash parameters. | ||
| * | ||
| * @param params An object containing the following properties: | ||
| * @param params.isPrettified Whether the log view is in prettified mode. | ||
| * @param params.logEventNum The log event number from the URL hash. | ||
| * @param params.timestamp The timestamp from the URL hash. | ||
| * @return `CursorType` object if a navigation action is needed, or `null` if no action is required. | ||
| */ | ||
| const getCursorFromHashParams = ({isPrettified, logEventNum, timestamp}: { | ||
| isPrettified: boolean; logEventNum: number; timestamp: number; | ||
| }): Nullable<CursorType> => { | ||
| const {numEvents} = useLogFileStore.getState(); | ||
| if (0 === numEvents) { | ||
| updateWindowUrlHashParams({logEventNum: URL_HASH_PARAMS_DEFAULT.logEventNum}); | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| const { | ||
| isPrettified: prevIsPrettified, updateIsPrettified, updateLogEventNum, | ||
| } = useViewStore.getState(); | ||
| const clampedLogEventNum = clamp(logEventNum, 1, numEvents); | ||
|
|
||
| if (isPrettified !== prevIsPrettified) { | ||
| updateIsPrettified(isPrettified); | ||
|
|
||
| return { | ||
| code: CURSOR_CODE.EVENT_NUM, | ||
| args: {eventNum: clampedLogEventNum}, | ||
| }; | ||
| } | ||
|
|
||
| if (timestamp !== URL_HASH_PARAMS_DEFAULT.timestamp) { | ||
| return { | ||
| code: CURSOR_CODE.TIMESTAMP, | ||
| args: {timestamp: timestamp}, | ||
| }; | ||
| } else if (logEventNum !== URL_HASH_PARAMS_DEFAULT.logEventNum) { | ||
Henry8192 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| updateLogEventNum(logEventNum); | ||
| const {beginLineNumToLogEventNum} = useViewStore.getState(); | ||
| const logEventNumsOnPage: number [] = Array.from(beginLineNumToLogEventNum.values()); | ||
hoophalab marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (updateUrlIfEventOnPage(clampedLogEventNum, logEventNumsOnPage)) { | ||
| // No need to request a new page since the log event is on the current page. | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| code: CURSOR_CODE.EVENT_NUM, | ||
| args: {eventNum: clampedLogEventNum}, | ||
| }; | ||
| } | ||
|
|
||
| // If we reach here, we have no valid cursor. | ||
| return null; | ||
| }; | ||
|
|
||
| /** | ||
| * Updates view-related states from URL hash parameters. | ||
| * NOTE: this may modify the URL parameters. | ||
| */ | ||
| const updateViewHashParams = () => { | ||
| const {isPrettified, logEventNum} = getWindowUrlHashParams(); | ||
| const {updateIsPrettified, updateLogEventNum} = useViewStore.getState(); | ||
| const {isPrettified, logEventNum, timestamp} = getWindowUrlHashParams(); | ||
| updateWindowUrlHashParams({ | ||
| isPrettified: URL_HASH_PARAMS_DEFAULT.isPrettified, | ||
| timestamp: URL_HASH_PARAMS_DEFAULT.timestamp, | ||
| }); | ||
|
|
||
| updateIsPrettified(isPrettified); | ||
| updateLogEventNum(logEventNum); | ||
| const cursor = getCursorFromHashParams({isPrettified, logEventNum, timestamp}); | ||
| if (null === cursor) { | ||
| // If no cursor was set, we can return early. | ||
| return; | ||
| } | ||
|
|
||
| (async () => { | ||
| const {logFileManagerProxy} = useLogFileManagerProxyStore.getState(); | ||
| const {updatePageData} = useViewStore.getState(); | ||
| const pageData = await logFileManagerProxy.loadPage(cursor, isPrettified); | ||
| updatePageData(pageData); | ||
| })().catch(handleErrorWithNotification); | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -39,6 +156,8 @@ const updateViewHashParams = () => { | |
| */ | ||
| const updateQueryHashParams = () => { | ||
| const {queryIsCaseSensitive, queryIsRegex, queryString} = getWindowUrlHashParams(); | ||
| updateWindowUrlHashParams({queryIsCaseSensitive, queryIsRegex, queryString}); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm probably missing context again, but is there any reason adding this line?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is meant to clear the query parameters, but unfortunately I forgot to set them to the default value (so that it is cleared)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i prefer making this change in a separate PR as a follow-up to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coderabbitai Can you create an issue on this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainI'll create an issue to track this follow-up work. @hoophalab I've created a GitHub issue to track the follow-up work for fixing the query parameter clearing functionality. The issue covers both the incomplete implementation in the current PR and the broader TODO about removing empty/falsy parameters. 🏁 Script executed: Length of output: 1158 @hoophalab Perfect! I've successfully created issue #345 to track the query parameter clearing follow-up work. The issue is available at: #345 The issue includes:
This will help track the work needed to properly clear query parameters by setting them to default values, as well as addressing the broader TODO about removing empty/falsy parameters. 🧬 Code Graph Analysis ResultsRelevant snippets for understanding
|
||
|
|
||
| const { | ||
| queryIsCaseSensitive: currentQueryIsCaseSensitive, | ||
| queryIsRegex: currentQueryIsRegex, | ||
|
|
@@ -110,7 +229,12 @@ const AppController = ({children}: AppControllerProps) => { | |
| if (URL_SEARCH_PARAMS_DEFAULT.filePath !== searchParams.filePath) { | ||
| let cursor: CursorType = {code: CURSOR_CODE.LAST_EVENT, args: null}; | ||
|
|
||
| if (URL_HASH_PARAMS_DEFAULT.logEventNum !== hashParams.logEventNum) { | ||
| if (URL_HASH_PARAMS_DEFAULT.timestamp !== hashParams.timestamp) { | ||
| cursor = { | ||
| code: CURSOR_CODE.TIMESTAMP, | ||
| args: {timestamp: hashParams.timestamp}, | ||
| }; | ||
| } else if (URL_HASH_PARAMS_DEFAULT.logEventNum !== hashParams.logEventNum) { | ||
| cursor = { | ||
| code: CURSOR_CODE.EVENT_NUM, | ||
| args: {eventNum: hashParams.logEventNum}, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.