refactor: Extract isPrettified from loadPage's parameters into a variable in LogFileManager (fixes #354).#362
Merged
hoophalab merged 5 commits intoy-scope:mainfrom Aug 11, 2025
Conversation
WalkthroughThe changes refactor how the prettification state ( Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant ViewStore
participant LogFileManagerProxy
participant LogFileManager
UI->>ViewStore: User toggles prettification
ViewStore->>LogFileManagerProxy: setIsPrettified(isPrettified)
LogFileManagerProxy->>LogFileManager: setIsPrettified(isPrettified)
UI->>ViewStore: Request page (loadPage)
ViewStore->>LogFileManagerProxy: loadPage(cursor)
LogFileManagerProxy->>LogFileManager: loadPage(cursor)
LogFileManager->>LogFileManager: Uses internal #isPrettified
LogFileManager-->>LogFileManagerProxy: PageData
LogFileManagerProxy-->>ViewStore: PageData
ViewStore-->>UI: Rendered page
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
src/services/LogFileManager/index.ts(4 hunks)src/services/LogFileManagerProxy.ts(1 hunks)src/stores/logFileStore.ts(1 hunks)src/stores/viewStore/createViewFilterSlice.ts(1 hunks)src/stores/viewStore/createViewPageSlice.ts(1 hunks)src/utils/url/urlHash.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewPageSlice.tssrc/stores/viewStore/createViewFilterSlice.tssrc/services/LogFileManager/index.tssrc/services/LogFileManagerProxy.tssrc/utils/url/urlHash.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#347
File: src/utils/math.ts:47-56
Timestamp: 2025-07-24T21:28:56.838Z
Learning: In the y-scope/yscope-log-viewer codebase, Henry8192 prefers not to introduce local variables to avoid parameter reassignment in binary search implementations, considering the additional constants unnecessary.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#107
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/Result.tsx:1-7
Timestamp: 2024-10-28T18:40:29.816Z
Learning: In the y-scope/yscope-log-viewer project, importing components via destructuring from 'mui/joy' is acceptable, as the package size impact has been analyzed and found acceptable.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#101
File: docs/dev-guide/building.md:18-18
Timestamp: 2024-10-24T01:18:27.752Z
Learning: In `docs/dev-guide/building.md` of the `yscope-log-viewer` project, it's acceptable to include bare URLs to keep them clickable, instead of wrapping them in backticks or converting them to Markdown links.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#196
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:55-58
Timestamp: 2025-03-03T18:10:02.451Z
Learning: In the y-scope/yscope-log-viewer repository, the team prefers using the `!` operator to flip boolean values (e.g., `!isRegex`) rather than the pattern `false == <expression>` that's mentioned in the coding guidelines.
📚 Learning: 2025-05-09T01:07:32.803Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewPageSlice.tssrc/stores/viewStore/createViewFilterSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2025-05-09T22:39:04.485Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewPageSlice.tssrc/stores/viewStore/createViewFilterSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2024-09-25T21:12:28.732Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewPageSlice.tssrc/services/LogFileManagerProxy.tssrc/utils/url/urlHash.ts
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewFilterSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2024-10-09T02:08:56.053Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/services/LogFileManager.ts:297-313
Timestamp: 2024-10-09T02:08:56.053Z
Learning: In the `#searchChunk` method of `LogFileManager.ts`, checking `typeof match.index === "number"` correctly handles matches at index 0, ensuring matches at the beginning of the string are not skipped.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewFilterSlice.tssrc/services/LogFileManager/index.tssrc/services/LogFileManagerProxy.ts
📚 Learning: 2025-05-17T05:13:39.947Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Applied to files:
src/stores/logFileStore.tssrc/stores/viewStore/createViewFilterSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2024-09-25T21:13:37.250Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Applied to files:
src/stores/viewStore/createViewPageSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2024-10-19T03:33:29.578Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/stores/viewStore/createViewFilterSlice.tssrc/utils/url/urlHash.ts
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-06-01T13:44:05.278Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
Applied to files:
src/utils/url/urlHash.ts
🧬 Code Graph Analysis (3)
src/stores/logFileStore.ts (1)
src/services/LogFileManager/index.ts (1)
cursor(443-477)
src/services/LogFileManagerProxy.ts (4)
src/services/LogFileManager/index.ts (1)
cursor(443-477)src/typings/worker.ts (2)
CursorType(108-108)PageData(111-111)src/services/decoders/JsonlDecoder/index.ts (1)
logLevelFilter(244-258)src/typings/logs.ts (1)
LogLevelFilter(51-51)
src/utils/url/urlHash.ts (2)
src/services/LogFileManagerProxy.ts (1)
setIsPrettified(60-63)src/services/LogFileManager/index.ts (2)
setIsPrettified(199-201)cursor(443-477)
🔇 Additional comments (10)
src/stores/viewStore/createViewPageSlice.ts (1)
142-142: LGTM! Parameter removal aligns with refactoring objective.The removal of the
isPrettifiedparameter fromloadPagecorrectly reflects the new architecture where prettification state is managed internally withinLogFileManager.src/stores/logFileStore.ts (1)
148-148: LGTM! Consistent parameter removal.The removal of the
isPrettifiedparameter fromloadPageis consistent with the refactoring to centralize prettification state management withinLogFileManager.src/stores/viewStore/createViewFilterSlice.ts (1)
31-31: LGTM! Consistent with filter method signature change.The removal of
isPrettifiedfrom the destructured state correctly reflects the updatedsetFiltermethod signature where prettification is now managed internally.src/services/LogFileManager/index.ts (4)
65-65: LGTM! Appropriate default value for prettification state.The private field initialization with
falseprovides a sensible default for the prettification state.
199-201: LGTM! Clean setter implementation.The
setIsPrettifiedmethod provides a clean interface for updating the internal prettification state.
242-242: LGTM! Method signature correctly updated.The removal of the
isPrettifiedparameter fromloadPagecorrectly implements the refactoring to use internal state management.
272-274: LGTM! Correct usage of internal state.The change from parameter to
this.#isPrettifiedcorrectly utilizes the internal state for prettification logic.src/services/LogFileManagerProxy.ts (3)
45-48: LGTM: Method signature correctly updated to remove prettification parameter.The removal of the
isPrettifiedparameter aligns with the refactoring objective to centralize prettification state management within theLogFileManager.
50-58: LGTM: Filter method correctly updated to match new architecture.The
setFiltermethod properly delegates to the updatedloadPagemethod without passing theisPrettifiedparameter, maintaining consistency with the refactored design.
60-63: LGTM: New setter method properly encapsulates prettification state management.The
setIsPrettifiedmethod follows the established patterns in the class by using#getLogFileManager()for error handling and properly delegates to the underlyingLogFileManager.
hoophalab
requested changes
Aug 8, 2025
Contributor
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
The change looks clean and nice, but I noticed a bug which probably isn't caused by this PR.
- Open any file
- Click isPrettified button
- Notice that the url isn't updated
- After the file is prettified, click any line in the text editor
- The output becomes "un-prettifed" because the url isn't updated
This is probably due to incorrect logic in updateViewHashParams and getCursorFromHashParams. Since we are already updating the isPrettifed logic in those two function, can we find a fix for this?
Contributor
Author
that was the bug I talked about yesterday, and I plan to fix it once this is merged. Still, if ypu think this is within the scope, I actually prefer to fix it here |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/utils/url/urlHash.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}
⚙️ CodeRabbit Configuration File
- Prefer
false == <expression>rather than!<expression>.
Files:
src/utils/url/urlHash.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-09-30T20:49:32.508Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-09-25T21:11:48.621Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/services/fileManager/utils.ts:43-59
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Avoid commenting on code that is only being moved and not modified in the PR.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-10-08T15:52:50.753Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#74
File: new-log-viewer/src/components/CentralContainer/Sidebar/ResizeHandle.tsx:77-82
Timestamp: 2024-09-28T02:32:08.882Z
Learning: Accessibility improvements, such as adding ARIA attributes to components like `ResizeHandle`, require broader discussion and are considered out of scope for small PRs.
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#347
File: src/utils/math.ts:47-56
Timestamp: 2025-07-24T21:28:56.838Z
Learning: In the y-scope/yscope-log-viewer codebase, Henry8192 prefers not to introduce local variables to avoid parameter reassignment in binary search implementations, considering the additional constants unnecessary.
📚 Learning: 2025-06-01T13:41:12.938Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/CentralContainer/Sidebar/SidebarTabs/SearchTabPanel/QueryInputBox.tsx:37-40
Timestamp: 2025-06-01T13:41:12.938Z
Learning: The `updateWindowUrlHashParams` function in `src/utils/url.ts` doesn't throw errors, so error handling is not needed when calling this function.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-09-25T21:12:28.732Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:105-133
Timestamp: 2024-09-25T21:12:28.732Z
Learning: In the `actions.ts` file, the `PAGE_TOP` and `PAGE_BOTTOM` actions are not intended to be handled in the `getPageNumCursorArgs` function.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-09-25T21:13:37.250Z
Learnt from: davemarco
PR: y-scope/yscope-log-viewer#76
File: new-log-viewer/src/utils/actions.ts:84-139
Timestamp: 2024-09-25T21:13:37.250Z
Learning: In the `getPageNumCursorArgs` function, variables `newPageNum` and `anchor` are always initialized when handling valid actions, so there is no risk of them being uninitialized.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-05-09T01:07:32.803Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/viewStore.ts:108-109
Timestamp: 2025-05-09T01:07:32.803Z
Learning: For the viewStore in the yscope-log-viewer project, it's more appropriate to suppress the max-lines-per-function ESLint rule than to split the store into slices due to the tight coupling and cohesive nature of its functions.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-10-19T03:33:29.578Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#94
File: new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx:99-118
Timestamp: 2024-10-19T03:33:29.578Z
Learning: In `new-log-viewer/src/components/CentralContainer/Sidebar/index.tsx`, when using `useEffect` to register window resize event handlers, it's acceptable to have an empty dependency array because the variables and functions used within the effect (`getPanelWidth`, `PANEL_CLIP_THRESHOLD_IN_PIXELS`, and `tabListRef`) are constants or refs declared outside the functional component and do not change.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-10-18T01:13:02.946Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#83
File: new-log-viewer/src/components/theme.tsx:59-61
Timestamp: 2024-10-18T01:13:02.946Z
Learning: In 'new-log-viewer/src/components/theme.tsx', a 1px focus ring is acceptable due to sufficient color contrast as per project preferences.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-05-27T18:09:40.038Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#173
File: src/contexts/UrlContextProvider.tsx:1-1
Timestamp: 2025-05-27T18:09:40.038Z
Learning: The UrlContextProvider in src/contexts/UrlContextProvider.tsx is planned to be eventually replaced and deleted, so refactoring efforts to reduce its file size should be avoided despite it being large enough to require eslint-disable max-lines.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-10-09T02:08:56.053Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#80
File: new-log-viewer/src/services/LogFileManager.ts:297-313
Timestamp: 2024-10-09T02:08:56.053Z
Learning: In the `#searchChunk` method of `LogFileManager.ts`, checking `typeof match.index === "number"` correctly handles matches at index 0, ensuring matches at the beginning of the string are not skipped.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2024-10-08T15:52:50.753Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#84
File: new-log-viewer/src/components/modals/SettingsModal/SettingsDialog.tsx:99-114
Timestamp: 2024-10-08T15:52:50.753Z
Learning: When suggesting to add validation for form inputs in `SettingsDialog.tsx`, the user considered it not related to the PR. In future reviews, ensure suggestions align with the PR's scope.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-06-01T13:40:12.222Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/Editor/index.tsx:138-139
Timestamp: 2025-06-01T13:40:12.222Z
Learning: In the yscope-log-viewer codebase, when using Zustand stores in React components, the preferred pattern is to use `getState()` for static setters that never change (like `setLogEventNum`) to avoid unnecessary subscriptions, while using hooks for actions that do more than just setting values. All store state variables should be declared at the beginning of the component for consistency and clear dependency overview.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-06-01T13:44:05.278Z
Learnt from: zzxthehappiest
PR: y-scope/yscope-log-viewer#286
File: src/components/AppController.tsx:40-43
Timestamp: 2025-06-01T13:44:05.278Z
Learning: In the y-scope/yscope-log-viewer codebase, when functions return invalid or default log event numbers, they should use 0 as this aligns with URL_HASH_PARAMS_DEFAULT.LOG_EVENT_NUM which is set to 0. This maintains consistency with the established default hash parameter pattern.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-05-17T05:13:39.947Z
Learnt from: junhaoliao
PR: y-scope/yscope-log-viewer#265
File: src/stores/logFileStore.ts:125-125
Timestamp: 2025-05-17T05:13:39.947Z
Learning: When refactoring Zustand state imports to prevent circular dependencies in the yscope-log-viewer project, the preferred pattern is to move state retrievals closest to where they're actually used rather than retrieving them at the beginning of the function.
Applied to files:
src/utils/url/urlHash.ts
📚 Learning: 2025-05-09T22:39:04.485Z
Learnt from: Henry8192
PR: y-scope/yscope-log-viewer#224
File: src/contexts/states/logFileStore.ts:72-79
Timestamp: 2025-05-09T22:39:04.485Z
Learning: In the log viewer application, silently ignoring calls to `handleExportChunk` when `logExportManager` is null is intentional, as the page is reloaded in this scenario anyway.
Applied to files:
src/utils/url/urlHash.ts
🧬 Code Graph Analysis (1)
src/utils/url/urlHash.ts (2)
src/services/LogFileManagerProxy.ts (1)
setIsPrettified(60-63)src/services/LogFileManager/index.ts (2)
setIsPrettified(199-201)cursor(443-477)
🔇 Additional comments (1)
src/utils/url/urlHash.ts (1)
41-41: Cursor choice on prettify mismatch looks correctUsing viewStore’s prevIsPrettified to detect a mismatch and emit an EVENT_NUM cursor is consistent with the new internalised prettify state. No issues here.
hoophalab
reviewed
Aug 8, 2025
hoophalab
approved these changes
Aug 8, 2025
Contributor
hoophalab
left a comment
hoophalab
left a comment
There was a problem hiding this comment.
the change is clean.
Validations:
isPrettified button works.
isPrettified url hash parameter is updated correctly.
Henry8192
changed the title
isPrettified from loadPage parameters as a variable in logFileManagerisPrettified from loadPage parameters as a variable in logFileManager (fixes #354).
hoophalab
changed the title
isPrettified from loadPage parameters as a variable in logFileManager (fixes #354).isPrettified from loadPage parameters as a variable in LogFileManager (fixes #354).
kirkrodrigues
approved these changes
Aug 11, 2025
Member
kirkrodrigues
left a comment
kirkrodrigues
left a comment
There was a problem hiding this comment.
Deferring to @hoophalab's review.
For the PR title, how about:
refactor: Extract `isPrettified` from `loadPage`'s parameters into a variable in `LogFileManager` (fixes #354).
isPrettified from loadPage parameters as a variable in LogFileManager (fixes #354).isPrettified from loadPage's parameters into a variable in LogFileManager (fixes #354).
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request refactors how the "prettified" (pretty-printed) state for log messages is managed across the log viewer application. Instead of passing the
isPrettifiedflag through multiple method calls, the state is now managed internally within theLogFileManagerclass. This simplifies method signatures and centralizes the state management for prettified logs.Refactoring and State Management:
Added a private
#isPrettifiedproperty and asetIsPrettifiedmethod to theLogFileManagerclass to manage prettified state internally, removing the need to passisPrettifiedas a parameter to methods likeloadPage.In
urlHash.ts, fixes isPrettified URL hash parameter gets cleared regardless of value #354.Updated
LogFileManagerProxyto provide asetIsPrettifiedmethod, and refactored itsloadPageandsetFiltermethods to no longer require theisPrettifiedargument, forwarding the prettified state toLogFileManageras needed.Store and View Logic Updates:
Refactored store slices (
logFileStore,viewStore/createViewPageSlice.ts, andviewStore/createViewFilterSlice.ts) to remove the need to pass theisPrettifiedparameter when loading pages or setting filters.URL and View Synchronization:
Updated URL utility logic to synchronize the prettified state via dedicated setter methods, ensuring the view and log manager are updated when the prettified state changes, instead of passing the flag through page load calls.
Checklist
breaking change.
Validation performed
Open log viewer with link: calhost:3010/?filePath=https://yscope.s3.us-east-2.amazonaws.com/sample-logs/yarn-ubuntu-resourcemanager-ip-172-31-17-135.log.1.clp.zst
isPrettifiedfrom URL hash parameters worksisPrettifiedtoggling works.Summary by CodeRabbit
New Features
Refactor