cli: remove author name from default annotate template, reorder fields

[yuja]

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

[ilyagr]

If this is the easiest fix in the short term, I think it's fine. I'll let others look at this before approving.

I'm hoping this won't be the final state though.

I have some thoughts about the future, but they are not blocking. The most actionable one is the one about dates, but I don't think it is mandatory.

---

How hard would it be to implement padding?

[yuja]

If this is the easiest fix in the short term, I think it's fine.

I believe so. I just want to make the default usable.

How hard would it be to implement padding?

I don't think it's difficult to implement a padding-only function, but we'll need to come up with a nicer syntax.

A padding function would have to support optional min/max widths, padding character, left/right/center alignment. For readability, it's nice if multiple texts can be inlined in a string literal (just like Rust's format!().) This means padding function will take another template mini language.

[ilyagr]

I'll just approve it then. But let's wait until tomorrow to merge.

[allonsy]

Just to catch myself up, is the purpose of this PR to remove the author because it messes with alignment?
I do like having the author included in some way in the annotate and its an important part of the change metadata.
However, it does make formatting a bit of a nightmare.
The issue with padding is it requires global context (meaning it needs to know all the input lines in order to format properly). We are also formatting information in a table like way meaning alignment is important for each column. This is a challenge, especially if the user inputs a custom template for file annotating. This is one of the reasons I decided not to deal with this in the original PR and save that for a later day

[yuja]

is the purpose of this PR to remove the author because it messes with alignment?

Yes, that's the main reason. Another reason is that author name can be long enough to make file contents invisible. So even if we include author, I want to truncate it up to 4-8 characters.

BTW, I've implemented padding/truncating function. If we'd like to include author in the default template, it can be something like pad(format_short_signature(author), min_width=8, max_width=8).

[allonsy]

is the purpose of this PR to remove the author because it messes with alignment?

Yes, that's the main reason. Another reason is that author name can be long enough to make file contents invisible. So even if we include author, I want to truncate it up to 4-8 characters.

BTW, I've implemented padding/truncating function. If we'd like to include author in the default template, it can be something like pad(format_short_signature(author), min_width=8, max_width=8).

Nice, I see no reason not to merge this PR as is. I can either add author manually with padding as mentioned or discuss in an issue if people feel like it is default-worthy

[yuja]

Do we like to include author name in the default annotation template?
Since padding function is landed, I can rewrite the template to be fully fixed widths.

[joyously]

I vote yes for showing the author and if anything needs to be cut, omit the commit ID.

[ilyagr]

My personal preference would be to include the email, trimmed to (say) 8-10 chars. We could try the trimmed name, but that will be annoying if we have more than one Alexander or Jean-Jacques one day.

We could also consider various ways to abbreviate names, but probably not in the first version. It'd also be cool to consider de-duplication, as in #4653 (comment), but that is also complicated.

[yuja]

Ok, updated the template to include hard-coded (change_id, commit_id, author, timestamp). PTAL.

[martinvonz]

I vote yes for showing the author and if anything needs to be cut, omit the commit ID.

+1. What do the rest of you think about removing the commit id?

[ilyagr]

I'm torn about the commit id.

Showing only the change id is a bit problematic if there are multiple commits with the same commit id, or if we blame hidden revisions. I'm not sure how to address this in a blame view. Update: One option would be to show the commit id instead of the change id in these cases, but that might be confusing.

I think it would furthermore be convenient for me to keep the change id, since I'd guess that if I'm blaming, I'm likely to be using other tools at the same time (GitHub, VS Code) that might be more familiar with commit ids.

That said, I agree that it's the least important of the things we currently show.

[ilyagr]

Ok, updated the template to include hard-coded (change_id, commit_id, author, timestamp). PTAL.

LGTM, thank you very much! I have some minor suggestions above, but we can experiment with them later as well. This is certainly better than what we had before.

[ilyagr]

By the way, there is nothing stopping us from providing a few prefabricated annotate template aliases that the user could swap between, is there? (Probably in a separate PR) Or do we not want the user to configure templates.annotate_commit_summary, even to choose between a few options?

Also, we probably want to allow something like jj file annotate -T minimal or even jj file annotate -T commit_id eventually, right? It seems pretty clear that options to show just the commit id and the line number and/or content would be good for scripting. We could also have a git-like template.

The one issue I see is that, ideally, the whole line including the line number and content would be templated, but I don't think it's a huge deal if it isn't.