Skip to content

templates: make op_summary in e.g. jj undo output more readable and colorful#4602

Merged
ilyagr merged 3 commits intomainfrom
ig/op_summary
Oct 14, 2024
Merged

templates: make op_summary in e.g. jj undo output more readable and colorful#4602
ilyagr merged 3 commits intomainfrom
ig/op_summary

Conversation

@ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Oct 7, 2024

This is meant to be a quick fix before I update the demos in the README. The primary reason to update them is the branch -> bookmark transition, but I noticed that jj undo's output is currently not very pretty.

I believe printing both the start and end times is excessive for a summary. For a long-running operation like jj describe with the editor open for a long time, the ending time seems more meaningful.

I was a bit torn on whether to use format_timestamp(self.time().end()) or self.time().end().ago(). The latter looks better (finished 2 seconds ago instead of finished 2024-10-07 15:46:03), but is less configurable and worse for dates long ago. In the future, we could add a format_op_summary_timestamp function and/or a template function that uses .ago() for recent dates and absolute dates for old dates.

Previous look:
Pasted image 20241006191123

Or in the demo:
image

New look:

image
Previous version image Pasted image 20241007143128

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 ilyagr marked this pull request as ready for review October 7, 2024 22:28
@ilyagr ilyagr changed the title templates: make op_summary more readable and colorful templates: make op_summary in e.g. jj undo output more readable and colorful Oct 7, 2024
@ilyagr ilyagr force-pushed the ig/op_summary branch 3 times, most recently from 2cc8459 to 4469c82 Compare October 13, 2024 02:09
ilyagr added a commit that referenced this pull request Oct 13, 2024
Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.

Add colon
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

ilyagr added a commit that referenced this pull request Oct 13, 2024
Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.

Add colon
ilyagr added a commit that referenced this pull request Oct 13, 2024
Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.

Add colon
@ilyagr
Copy link
Contributor Author

ilyagr commented Oct 13, 2024

I'll keep this unmerged for a few hours in case somebody has a comment or wants me to wait.

ilyagr added a commit that referenced this pull request Oct 13, 2024
Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.

Add colon
ilyagr added a commit that referenced this pull request Oct 14, 2024
Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.
Copy link
Contributor

@yuja yuja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Added colons to make it seem less like an English sentence, see
#4602 (comment)

I believe printing both the start and end times is excessive for a
summary. For now, I have it print just the start time for consistency.
I intend to change it to print the ending time later.

I was a bit torn on whether to use `format_timestamp(self.time().start())`
or `self.time().start().ago()`. The latter looks better, but is less
configurable and worse for dates long ago. In the future, we could add a
`format_op_summary_timestamp` function and/or a template function that
uses `.ago()` for recent dates and absolute dates for old dates.
@ilyagr ilyagr enabled auto-merge (rebase) October 14, 2024 02:57
@ilyagr ilyagr merged commit e761844 into main Oct 14, 2024
@ilyagr ilyagr deleted the ig/op_summary branch October 14, 2024 03:06
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.

2 participants