Add command jj annotate/blame #4176

allonsy · 2024-07-29T15:51:46Z

This change adds a new annotate command (in git this is blame).
In order to annotate a file, call: jj annotate <FILE_PATH>.
In response, jj will traverse the commit graph, trying to find the source of the change for each line.
The output is something similar to this:

>>> jj annotate file.txt
tqqpzkzp bd94545f Alec Snyder 2024-07-29 18:39:13 1: this is an initial line
nqvnwumz b3a07c79 Alec Snyder 2024-07-29 18:39:35 2: this line is commit 1
wlpkktol 5f7de224 Alec Snyder 2024-07-29 18:40:01 3: this line is commit 2

I've also added a new module to jj_lib which exposes a function get_annotation_for_file which
returns a line by line evaluation of the the given file in a more general interface.

A few caveats

directories aren't supported
conflicted files aren't supported as I'm not exactly sure what to print out in these cases.

Checklist

If applicable:

I have updated CHANGELOG.md
I have updated the documentation (README.md, docs/, demos/)
I have updated the config schema (cli/src/config-schema.json)
I have added tests to cover my changes

allonsy · 2024-07-29T15:58:32Z

As far as the test cases I've thrown at it, it matches with git blame but if anyone finds a counterexample, I'm happy to take a look and see where my bugs are

PhilipMetzger

Just a initial comment, it should also mention that this starts work on #2988.

CHANGELOG.md

cli/tests/[email protected]

lib/src/annotate.rs

PhilipMetzger

Here's a second review.

lib/src/annotate.rs

cli/tests/test_annotate_command.rs

ilyagr · 2024-07-29T23:59:57Z

conflicted files aren't supported as I'm not exactly sure what to print out in these cases.

I think that it should figure out which side of the conflict comes from which parent of the commit. Any sides that don't come from a parent (e.g. after a rebase) can be attributed as being added to the commit itself.

Infrastructure related to #4062 would be helpful here.

Of course, none of this is necessary for a first version of annotate functionality.

lib/src/annotate.rs

martinvonz · 2024-07-30T17:54:53Z

Thanks for working on this! I have not looked at the implementation yet, but I'll just quickly note that I think a TUI for the functionality would be really useful. I quite often end up trying to find where the previous version of a line came from, and quite often I end up repeating that process several times. GitHub has a "Blame prior to change abc123..." button for each line (or span of lines) for that. It's probably not a big deal to redo the calculation when the user asks for that option, but ideally we could reuse some of the information we already calculated.

allonsy · 2024-07-30T18:05:57Z

Thanks for working on this! I have not looked at the implementation yet, but I'll just quickly note that I think a TUI for the functionality would be really useful. I quite often end up trying to find where the previous version of a line came from, and quite often I end up repeating that process several times. GitHub has a "Blame prior to change abc123..." button for each line (or span of lines) for that. It's probably not a big deal to redo the calculation when the user asks for that option, but ideally we could reuse some of the information we already calculated.

That's a nice idea. I'm hoping to merge this in and maybe in a future PR I can implement a TUI? Unless you think implementing a TUI would radically change things enough to not make it worth it to merge this in now?

martinvonz · 2024-07-30T18:24:31Z

I'm hoping to merge this in and maybe in a future PR I can implement a TUI?

Sounds good. (And I didn't mean to ask you to create a TUI, but sounds great if you're interested in doing that.)

Unless you think implementing a TUI would radically change things enough to not make it worth it to merge this in now?

Nope. I just mentioned it in case it would help any future work in this area.

allonsy · 2024-07-30T18:25:48Z

conflicted files aren't supported as I'm not exactly sure what to print out in these cases.

I think that it should figure out which side of the conflict comes from which parent of the commit. Any sides that don't come from a parent (e.g. after a rebase) can be attributed as being added to the commit itself.

Infrastructure related to #4062 would be helpful here.

Of course, none of this is necessary for a first version of annotate functionality.

I agree with this for conflicts, I initially tried this but gave up for the first iteration as currently the conflict writers (lib/src/conflict.rs) doesn't have the commit information. So I'd have to forward that through the callers. Definitely doable but a little out of scope for this PR I think

cli/src/commands/annotate.rs

martinvonz

I'm not done reviewing yet, but here are my comments so far

CHANGELOG.md

cli/src/commands/annotate.rs

lib/src/annotate.rs

PhilipMetzger

LG to me after this round, but you should wait for a final approval from either Martin or Yuya.

CHANGELOG.md

cli/src/commands/mod.rs

lib/src/annotate.rs

CHANGELOG.md

cli/src/commands/annotate.rs

cli/src/commands/mod.rs

lib/src/annotate.rs

cli/src/commands/annotate.rs

lib/src/annotate.rs

PhilipMetzger · 2024-10-10T18:37:22Z

What are the remaining blockers here? As I'd really like to finally have a annotate command, even if it is not perfect yet.

yuja · 2024-10-11T00:34:21Z

What are the remaining blockers here? As I'd really like to finally have a annotate command, even if it is not perfect yet.

This and some related changes?
#4176 (comment)

I can rewrite the implementation as a follow up if he's okay.

allonsy · 2024-10-11T13:13:39Z

I was working on a few different implementations, none of which I was really happy with so I'm going to fix it up to a state that I liked over the next two days and then we can go over final review

…

On Fri, Oct 11, 2024, 03:34 Yuya Nishihara ***@***.***> wrote: What are the remaining blockers here? As I'd really like to finally have a annotate command, even if it is not perfect yet. This and some related changes? #4176 (comment) <#4176 (comment)> I can rewrite the implementation as a follow up if he's okay. — Reply to this email directly, view it on GitHub <#4176 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABM6VFBXCXZ3IGSDUUUUUQ3Z24MKHAVCNFSM6AAAAABLUSBBUWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWGMYDEMBZGM> . You are receiving this because you authored the thread.Message ID: ***@***.***>

allonsy · 2024-10-13T17:19:51Z

I think the last big issue remaining is command name.
Its between raw jj annotate or jj file annotate I think file annotate makes the most sense at this point right?
@yuja
@PhilipMetzger

yuja · 2024-10-14T02:55:40Z

I think the last big issue remaining is command name. Its between raw jj annotate or jj file annotate I think file annotate makes the most sense at this point right?

Yes, I would go with file annotate (and add annotate -> file annotate alias if we find it's the common requirement.)

yuja

Thanks.

CHANGELOG.md

cli/src/commands/annotate.rs

lib/src/annotate.rs

yuja

Thanks!

CHANGELOG.md

lib/src/annotate.rs

A new module is added to jj_lib which exposes a function get_annotation_for_file. This annotates the given file line by line with commit information according to the commit that made the most recent change to the line. Similarly, a new command is added to the CLI called `jj file annotate` which accepts a file path. It then prints out line by line the commit information for the line and the line itself. Specific commit information can be configured via the templates.annotate_commit_summary config variable

allonsy · 2024-10-15T09:44:38Z

@yuja addressed the last round of quick fixes. Anything more before merging?

yuja · 2024-10-15T11:14:09Z

Anything more before merging?

No. Thanks for updating it quickly.

PhilipMetzger reviewed Jul 29, 2024

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

allonsy force-pushed the annotate branch from 7d8d414 to e1aa066 Compare July 29, 2024 16:04

joyously reviewed Jul 29, 2024

View reviewed changes

cli/tests/[email protected] Outdated Show resolved Hide resolved

joyously reviewed Jul 29, 2024

View reviewed changes

lib/src/annotate.rs Outdated Show resolved Hide resolved

PhilipMetzger requested changes Jul 29, 2024

View reviewed changes

ilyagr reviewed Jul 30, 2024

View reviewed changes

lib/src/annotate.rs Outdated Show resolved Hide resolved

ilyagr reviewed Jul 30, 2024

View reviewed changes

lib/src/annotate.rs Show resolved Hide resolved

yuja reviewed Jul 30, 2024

View reviewed changes

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

allonsy force-pushed the annotate branch 3 times, most recently from 8090b5f to f62815a Compare July 30, 2024 18:40

yuja reviewed Jul 31, 2024

View reviewed changes

cli/src/commands/annotate.rs Outdated Show resolved Hide resolved

allonsy force-pushed the annotate branch 2 times, most recently from 684840d to ae48961 Compare August 6, 2024 16:55

martinvonz reviewed Aug 6, 2024

View reviewed changes

yuja reviewed Aug 7, 2024

View reviewed changes

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

PhilipMetzger approved these changes Aug 7, 2024

View reviewed changes

allonsy force-pushed the annotate branch 4 times, most recently from b446272 to 4c5831a Compare August 7, 2024 17:15

yuja reviewed Aug 8, 2024

View reviewed changes

allonsy force-pushed the annotate branch 2 times, most recently from 176b802 to 6eaff71 Compare August 13, 2024 18:11

yuja reviewed Aug 14, 2024

View reviewed changes

martinvonz mentioned this pull request Aug 14, 2024

Explaining diffs of Merge<T> (structured conflicts part 1) #4259

Draft

1 task

allonsy force-pushed the annotate branch 7 times, most recently from 843e419 to 407da8d Compare August 28, 2024 19:18

yuja reviewed Aug 29, 2024

View reviewed changes

lib/src/annotate.rs Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

allonsy force-pushed the annotate branch from 407da8d to 79e6ab2 Compare September 2, 2024 17:38

allonsy force-pushed the annotate branch from 79e6ab2 to 1915c88 Compare October 13, 2024 17:16

allonsy force-pushed the annotate branch from 1915c88 to 2df38da Compare October 13, 2024 17:45

yuja reviewed Oct 14, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

cli/src/commands/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

lib/src/annotate.rs Show resolved Hide resolved

lib/src/annotate.rs Outdated Show resolved Hide resolved

allonsy force-pushed the annotate branch from 2df38da to 2545c9f Compare October 14, 2024 17:37

yuja approved these changes Oct 15, 2024

View reviewed changes

allonsy force-pushed the annotate branch 3 times, most recently from e9bbedf to 3fea76c Compare October 15, 2024 09:38

allonsy force-pushed the annotate branch from 3fea76c to 420e5d9 Compare October 15, 2024 09:41

allonsy merged commit 470275b into jj-vcs:main Oct 15, 2024
18 checks passed

allonsy deleted the annotate branch October 15, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add command jj annotate/blame #4176

Add command jj annotate/blame #4176

allonsy commented Jul 29, 2024

allonsy commented Jul 29, 2024

PhilipMetzger left a comment

PhilipMetzger left a comment

ilyagr commented Jul 29, 2024

martinvonz commented Jul 30, 2024

allonsy commented Jul 30, 2024

martinvonz commented Jul 30, 2024

allonsy commented Jul 30, 2024

martinvonz left a comment

PhilipMetzger left a comment •

edited

Loading

PhilipMetzger commented Oct 10, 2024

yuja commented Oct 11, 2024

allonsy commented Oct 11, 2024 via email

allonsy commented Oct 13, 2024

yuja commented Oct 14, 2024

yuja left a comment

yuja left a comment

allonsy commented Oct 15, 2024

yuja commented Oct 15, 2024

Add command jj annotate/blame #4176

Add command jj annotate/blame #4176

Conversation

allonsy commented Jul 29, 2024

Checklist

allonsy commented Jul 29, 2024

PhilipMetzger left a comment

Choose a reason for hiding this comment

PhilipMetzger left a comment

Choose a reason for hiding this comment

ilyagr commented Jul 29, 2024

martinvonz commented Jul 30, 2024

allonsy commented Jul 30, 2024

martinvonz commented Jul 30, 2024

allonsy commented Jul 30, 2024

martinvonz left a comment

Choose a reason for hiding this comment

PhilipMetzger left a comment • edited Loading

Choose a reason for hiding this comment

PhilipMetzger commented Oct 10, 2024

yuja commented Oct 11, 2024

allonsy commented Oct 11, 2024 via email

allonsy commented Oct 13, 2024

yuja commented Oct 14, 2024

yuja left a comment

Choose a reason for hiding this comment

yuja left a comment

Choose a reason for hiding this comment

allonsy commented Oct 15, 2024

yuja commented Oct 15, 2024

PhilipMetzger left a comment •

edited

Loading