Skip to content
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

templates: respect format_short_signature() in builtin_log_oneline #4714

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

neongreen
Copy link
Contributor

I noticed that jj log -T builtin_log_oneline wasn't respecting my format_short_signature() settings.

This is a slightly breaking change -- people's oneline logs will become longer if they use the default format_short_signature(). Perhaps there should be a new override, eg. format_tiny_signature()?

Checklist

not updating tests/changelog yet since idk if the PR will be merged

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

Copy link

google-cla bot commented Oct 25, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@neongreen
Copy link
Contributor Author

@ilyagr I just noticed that you also want to have format_very_short_signature(). If it existed, I suppose it would be used both in jj annotate and in builtin_log_oneline?

@ilyagr
Copy link
Contributor

ilyagr commented Oct 26, 2024

I don't have a strong opinion, but whether or not we decide to show the author in the default annotation template (as discussed in #4653), Emily's idea of a format_very_short_signature that's used here and in a (possibly non-default) annotation template seems good to me. We could do this at the same time as or shortly after such an annotation template exists.

For example, in #4653 (comment), I was thinking that username@ might look better than just username, and format_very_short_signature would make that consistent (again, regardless of whether this is a new default or a user's customization).

Another possible value for format_very_short_signature might be truncate_end(email, 12), though then you'd have to adjust the annotation template to use the same padding whenever you change the number.

In fact, one could argue that this template already effectively exists, it's just unnamed and present in only one place.


I personally like formalizing format_very_short_signature slightly better than this PR's original idea using format_short_signature in the oneline template, since the default will become verbose, but either could make sense. From the point of view where we effectively already having the third format_..._signature template (without a name and used in one place), there is an upside to this PR removing it.

I don't use the oneline template enough to have a strong opinion.

@neongreen
Copy link
Contributor Author

I’m in the same boat as @emilazy:

I dislike how we have an entire field in commits that represents an appropriate human‐readable identifier to use for their authors and committers but ignore it to just show email addresses instead, […] Of course I’m biased since the local‐parts of my email addresses don’t identify me at all.

So, it’s not the default I particularly care about — just the ability to customize the name display, without having to copy templates into my config and keep up with future jj changes.

@ilyagr
Copy link
Contributor

ilyagr commented Oct 26, 2024

In that case, it sounds like creating a "very short signature" template alias will help you now, will work for a future annotation template, and will be a noop for everybody else. That sounds good to me, even if we later decide to change our mind and, say, unify two of the signature aliases or change the default "very short" template.

So, let's do that unless we hear another opinion?

@emilazy
Copy link
Contributor

emilazy commented Oct 26, 2024

At this rate, perhaps we could simply hard‐code it to output "Emily".

@ilyagr
Copy link
Contributor

ilyagr commented Oct 28, 2024

Just to be clear, I'm waiting for you to adjust the PR and I'm assuming you just haven't gotten around to it yet (which is totally fine!). But let me know if you're waiting on something instead.

@neongreen neongreen marked this pull request as draft October 28, 2024 07:30
@neongreen
Copy link
Contributor Author

Yeah, just haven’t gotten around to it; I’ll convert it into draft for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants