internal/parser: get revision ids with template prefixes by baggiiiie · Pull Request #372 · idursun/jjui

baggiiiie · 2025-11-21T10:20:31Z

summary

use jj log -T separate() to reliably fetch revision ChangeID, CommitID, IsDivergent, replacing current "best guess" effort

changes

As proposed in #358, using jj's native support (built-in change_id/commit_id/divergent) to get ChangeID and commitID would be more preferable than current "best guess" by checking log output sequence in order, which breaks when a bookmark is "HexLike" and "??".

The implementation of the proposal uses jj log's -T template option, option for -T is separate('', $prefixes, $template), where $prefixes is change_id.shortest(), commit_id.shortest(), divergent and $template is the user-configured template, i.e., config.Current.Revisions.Template

This way, ChangeID, CommitID, and IsDivergent for a revision are retrieved reliably, and the prefixes are removed after prefix parsing before rendering to the ui.

Fixes #358
Fixes #228

tests

updated existing tests in test/log_parser_test.go to include $change_id, $commit_id, $is_divergent in test data

others

existing test TestParser_Parse_NoCommitId with testdata/no-commit-id.log has commit id, though the test name indicates otherwise, unsure what the purpose of this test is

internal/parser/streaming_log_parser.go

test/log_parser_test.go

internal/parser/row.go

vic · 2025-11-21T11:32:14Z

haven't tested, but changes lgtm, this is awesome @baggiiiie 🎉 👏

baggiiiie · 2025-11-21T12:27:38Z

@vic thanks!
give it go too

idursun · 2025-11-21T13:13:24Z

Hey @baggiiiie thanks for the work. I briefly skimmed the changes and pulled your branch and gave it a run and it looks like below and I don't think this is intentional

and looks like this with the short id format

baggiiiie · 2025-11-21T14:07:56Z

@idursun oh wow i have been using this build for a few days and didn't bump into that issue at all
the expected output should look exactly the same as before this change

do you have a config for config.Current.Revisions.Template? and could you paste a image of the ui before this change for comparison?

idursun · 2025-11-21T14:57:16Z

do you have a config for config.Current.Revisions.Template?

No, that's for folks to override the default jj template. I don't have an override.

before:

after:

baggiiiie · 2025-11-21T16:18:48Z

@idursun hmmm i do not have that issue at all

hey @vic, would u be able to try out and see if you have any issue?

below screenshot and screen recording are captured with 'format_short_id(id)' = 'id.shortest()' alias in jj/config.toml

see the recording for a fresh clone from this branch & go build:

vic · 2025-11-21T16:27:44Z

@baggiiiie, sure!

Looks similar here, I don't have config.Current.Revisions.Template set, but I do have format_short_id alias

baggiiiie · 2025-11-22T05:08:01Z

hey @vic thanks for getting back and providing your config!

unfortunately, i have no lucky reproducing the bug after hours, attempting with all following templates and any potentially suspicious ones in your config

              default-command = [
                "log" 
                "--no-pager" 
                "--reversed" 
                "--stat" 
                "--template" 
                "builtin_log_compact_full_description"
                "--limit"
                "3"
              ];

            template-aliases = {
              "format_short_id(id)" = "id.shortest().upper()"; # default is shortest(12)
              "format_short_change_id(id)" = "format_short_id(id)";
              "format_short_signature(signature)" = "signature.email()";
              "format_timestamp(timestamp)" = "timestamp.ago()";

i pushed a new branch (yc/parsing-debug) in my fork with relevant debug loggings and a fix for ace jump, it would be truly appreciated if you could run it with DEBUG=1 and send over the debug.log 🙏🏼🙏🏼

vic · 2025-11-22T06:13:20Z

@baggiiiie using yc/parsing-debug branch and DEBUG=1 dumped this debug.log. Hope it is helpful.

idursun · 2025-11-22T18:06:15Z

@baggiiiie here's my debug.log

baggiiiie · 2025-11-23T16:36:50Z

@vic @idursun thank you guys for the help, much appreciated!! 🫶🏼🫶🏼

turned out it's my own config that's making things weird >:

i have added a Templates field to JJConfig, which is used to load jj config list --include-defaults --ignore-working-copy, and newly added Templates now saves templates.log and is used to run Log command

also, as ChangeID/CommitID are now fetched with jj log with .shortest(), they are always lowercase.
there might be a case where user uses uppercase for ui display (thanks @vic for your config!), and this breaks ace jump highlighting. this PR now has a fix for it.

all change details are in each commit's messages. thank yall again for helping out!

idursun · 2025-11-23T21:32:53Z

Thanks @baggiiiie this is looking much better now.

However, I noticed that evolog view is broken. It is using the the parser as revisions (the structure is pretty much the same) but I guess because evolog doesn't use a template now, the view is broken in the same way revisions was.

The recent parser changes (PR idursun#372) require template prefixes (change_id, commit_id, divergent) to be present in the output for proper parsing. Updated Evolog() function to use the same template prefix approach as Log() to fix evolog view breaking. Changes: - Added jjTemplate parameter to Evolog() function - Apply template prefix wrapping to evolog output - Updated evolog operation to pass JJConfig template - Updated test to handle new function signature - Initialized JJConfig in test context to prevent nil pointer errors Fixes the evolog breaking issue reported in PR idursun#372

As proposed in idursun#358, using `jj`'s native support (built-in `change_id/commit_id/divergent`) to get ChangeID and commitID would be more preferable than current "best guess" by checking log output sequence in order, which breaks when a bookmark is "HexLike" and "??". The implementation of the proposal uses `jj log`'s `-T` template option, option for `-T` is `separate('', $prefixes, $template)`, where `$prefixes` is `change_id.shortest(), commit_id.shortest(), divergent` and `$template` is the user-configured template, i.e., `config.Current.Revisions.Template` This way, ChangeID, CommitID, and IsDivergent for a revision are retrieved *reliably*, and the prefixes are removed after prefix parsing before rendering to the ui. Fixes idursun#358 Fixes idursun#228

As ChangeID and CommitID are fetched with jj's native template now, what's being fetched is always `change_id/commit_id.shortest()`, which is in lower case and might not be the same as what's displayed in the UI (user can configure it to be uppercase) Hence, introducing this change to make ace jump highlighting case insensitive

This commit adds jj's native templates in the jj log command with a `Templates` field added to the JJConfig jj log now accepts an optional jjTemplate parameter and fall back to jj's built-in templates if no custom template is provided

baggiiiie · 2025-11-24T13:32:52Z

@idursun patched the same prefixes to evolog command:

separate("",  commit.change_id().shortest(), commit.commit_id().shortest(), commit.divergent(), builtin_evolog_compact)

as jj now only has one default template for evolog, which is builtin_evolog_compact, i didn't use any config to fetch templates.evolog from jj config list

idursun

Nice work! Thanks.

Seems to be working fine for me now. We can always follow it up with a fix if we see something is broken.

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [idursun/jjui](https://github.com/idursun/jjui) | patch | `v0.9.6` -> `v0.9.7` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>idursun/jjui (idursun/jjui)</summary> ### [`v0.9.7`](https://github.com/idursun/jjui/releases/tag/v0.9.7) [Compare Source](idursun/jjui@v0.9.6...v0.9.7) 🎉 This release enhances performance and introduces stability improvements in log parsing and command execution. It also takes back some of the stability by adding basic mouse support. #### Features ##### 📈 Performance Improvements Implemented frame-rate limited rendering (capped at 120 FPS) to significantly improve application performance by deferring view generation until the next frame tick. This addresses the slowest path in the application - view generation - making it much more responsive ##### 🐭 Mouse Support - Clickable and scrollable revisions view (excluding operations) ([#391](idursun/jjui#391)) - Clickable and scrollable op log view - Draggable and scrollable (vertically + horizontally) preview pane - Scrollable diff window - Replaced custom viewport with bubbles/viewport for more responsive rendering ([#396](idursun/jjui#396)) <https://github.com/user-attachments/assets/7cbad275-713e-4fa3-8d61-4e86c301ab0c> ##### 🔎 Preview - Replaced surrounding border with divider for more preview space ([#396](idursun/jjui#396)) ##### Operation Log (oplog) - Added `jj op revert` functionality with `R` key binding ([#400](idursun/jjui#400)) ##### Revset Handling - Pressing up arrow in empty revset field now sets the current revset ([#284](idursun/jjui#284)) - Fixed mismatch where empty revset input would use config default instead of session default from `-r` flag. Now correctly updates `CurrentRevset` to session default instead of empty string ([#399](idursun/jjui#399)) ##### Details View - Allow quitting from details view when quit key is pressed (e.g. `q`) #### Bug Fixes ##### Rendering Issues - Fixed double rendering of inline describe content when next line contains only revision line. Added `revisionLineRendered` tracking flag to properly sequence the description overlay rendering ([#403](idursun/jjui#403), [#369](idursun/jjui#369)) - Fixed viewport adjustment when PageDown moves cursor below viewport. The renderer now continues rendering until it reaches the focused item, ensuring proper viewport adjustment ([#395](idursun/jjui#395)) - Fixed PageUp/Down navigation at top and bottom of revset when less than one page remains. Now includes early return with feedback message when already at boundary ([#387](idursun/jjui#387), [#386](idursun/jjui#386)) - Removed incorrect space trimming in renderer ([#393](idursun/jjui#393)) ##### Log Processing - Applied partial fixes to prevent out-of-order row updates in log streamer ##### Details View - Handle cases where conflict markers span multiple lines ([#398](idursun/jjui#398)) - Ignore virtual selection on refresh ([#381](idursun/jjui#381)) ##### Operation Log (oplog) - Fixed an issue in operation ID detection ([#380](idursun/jjui#380), [#377](idursun/jjui#377)) ##### Command Execution - Added a mechanism for restoring failed commands to input field, allowing retries without retyping ([#392](idursun/jjui#392)) ##### Template System - Enhanced `jj log` parsing using native template prefixes for better change ID and commit ID detection. Fixes issues when bookmarks are "HexLike" ([#358](idursun/jjui#358), [#228](idursun/jjui#228), [#372](idursun/jjui#372)) #### What's Changed - Remove teatest package and simplify tests by [@idursun](https://github.com/idursun) in [#379](idursun/jjui#379) - internal/parser: get revision ids with template prefixes by [@baggiiiie](https://github.com/baggiiiie) in [#372](idursun/jjui#372) - fix(oplog): improve operation id detection by [@idursun](https://github.com/idursun) in [#380](idursun/jjui#380) - refactor: serialise command execution by [@idursun](https://github.com/idursun) in [#378](idursun/jjui#378) - Revert "refactor: serialise command execution" by [@idursun](https://github.com/idursun) in [#383](idursun/jjui#383) - revisions: fix scrolling at the top and bottom of revset by [@baggiiiie](https://github.com/baggiiiie) in [#387](idursun/jjui#387) - refactor: introduce and implement common.Model by [@idursun](https://github.com/idursun) in [#384](idursun/jjui#384) - refactor: Replace custom cell buffer implementation with `cellbuf` package by [@idursun](https://github.com/idursun) in [#388](idursun/jjui#388) - refactor: use simple layout functions to lay out the main UI by [@idursun](https://github.com/idursun) in [#389](idursun/jjui#389) - Coming back to previous state when exec command failed by [@ArnaudBger](https://github.com/ArnaudBger) in [#392](idursun/jjui#392) - feat: add basic mouse support by [@idursun](https://github.com/idursun) in [#391](idursun/jjui#391) - list/renderer: fix viewport adjustment on PageDown by [@baggiiiie](https://github.com/baggiiiie) in [#395](idursun/jjui#395) - Make preview horizontally scrollable by [@idursun](https://github.com/idursun) in [#396](idursun/jjui#396) - revset: fix revset not using default when empty by [@baggiiiie](https://github.com/baggiiiie) in [#399](idursun/jjui#399) - operation: add op log revert by [@baggiiiie](https://github.com/baggiiiie) in [#400](idursun/jjui#400) - revision: fix double rendering of inline describe content by [@baggiiiie](https://github.com/baggiiiie) in [#403](idursun/jjui#403) - refactor: replace usages of scattered width/height pairs with `ViewNode` by [@idursun](https://github.com/idursun) in [#401](idursun/jjui#401) - describe: catch cursor blinking to avoid unnecessary rendering by [@baggiiiie](https://github.com/baggiiiie) in [#404](idursun/jjui#404) #### New Contributors - [@ArnaudBger](https://github.com/ArnaudBger) made their first contribution in [#392](idursun/jjui#392) **Full Changelog**: <idursun/jjui@v0.9.6...v0.9.7> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Previously, in PR idursun#372, ChangeID/CommitID/IsDivergent is added to jj log/evolog commands to allow fetching these revision metadata natively with jj's template. However, the change didn't take into consideration of different color formatting of ChangeID/CommitID/IsDivergent and treated each of them as different `screen.Segment`s. When user configures them to be the same color, ChangeID/CommitID/IsDivergent are actually returned as one chunk by jj and it becomes one `screen.Segment`. Hence the previous change breaks. This commit updates `ChangeID/CommitID/IsDivergent` prefixes with jj's `stringify` method, allowing them to be returned by jj consistently as one single chunk of text without color escape sequence. this way, `ChangeID/CommitID/IsDivergent` are always treated as one `screen.Segment` in jjui. Also, `_PREFIX:` is added to the template for easier separation and parsing of prefixes. Fixes idursun#411

Previously, in PR #372, ChangeID/CommitID/IsDivergent is added to jj log/evolog commands to allow fetching these revision metadata natively with jj's template. However, the change didn't take into consideration of different color formatting of ChangeID/CommitID/IsDivergent and treated each of them as different `screen.Segment`s. When user configures them to be the same color, ChangeID/CommitID/IsDivergent are actually returned as one chunk by jj and it becomes one `screen.Segment`. Hence the previous change breaks. This commit updates `ChangeID/CommitID/IsDivergent` prefixes with jj's `stringify` method, allowing them to be returned by jj consistently as one single chunk of text without color escape sequence. this way, `ChangeID/CommitID/IsDivergent` are always treated as one `screen.Segment` in jjui. Also, `_PREFIX:` is added to the template for easier separation and parsing of prefixes. Fixes #411

baggiiiie commented Nov 21, 2025

View reviewed changes

internal/parser/streaming_log_parser.go Show resolved Hide resolved

test/log_parser_test.go Show resolved Hide resolved

internal/parser/row.go Show resolved Hide resolved

baggiiiie changed the title ~~refactor(parser): get revision ids with template prefixes~~ internal/parser: get revision ids with template prefixes Nov 21, 2025

baggiiiie force-pushed the yc/test-u branch from fbf0115 to e3abb8f Compare November 21, 2025 10:35

baggiiiie force-pushed the yc/test-u branch from e3abb8f to e790a37 Compare November 23, 2025 16:25

baggiiiie added 3 commits November 24, 2025 20:12

baggiiiie force-pushed the yc/test-u branch from e790a37 to 02b5d0e Compare November 24, 2025 13:25

idursun approved these changes Nov 24, 2025

View reviewed changes

idursun merged commit 434b594 into idursun:main Nov 24, 2025
3 checks passed

baggiiiie deleted the yc/test-u branch November 25, 2025 00:16

BrewTestBot mentioned this pull request Dec 3, 2025

jjui 0.9.7 Homebrew/homebrew-core#256845

Merged

baggiiiie mentioned this pull request Dec 7, 2025

parser: stringify log/evolog prefixes to be color agnostic #413

Merged

Conversation

baggiiiie commented Nov 21, 2025

summary

changes

tests

others

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vic commented Nov 21, 2025

Uh oh!

baggiiiie commented Nov 21, 2025

Uh oh!

idursun commented Nov 21, 2025

Uh oh!

baggiiiie commented Nov 21, 2025

Uh oh!

idursun commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

baggiiiie commented Nov 21, 2025

Uh oh!

vic commented Nov 21, 2025

Uh oh!

baggiiiie commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vic commented Nov 22, 2025

Uh oh!

idursun commented Nov 22, 2025

Uh oh!

baggiiiie commented Nov 23, 2025

Uh oh!

idursun commented Nov 23, 2025

Uh oh!

baggiiiie commented Nov 24, 2025

Uh oh!

idursun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

idursun commented Nov 21, 2025 •

edited

Loading

baggiiiie commented Nov 22, 2025 •

edited

Loading