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

FR: have jj log wrap text based on terminal width #1043

Closed
avamsi opened this issue Jan 15, 2023 · 2 comments
Closed

FR: have jj log wrap text based on terminal width #1043

avamsi opened this issue Jan 15, 2023 · 2 comments
Labels
polish🪒🐃 Make existing features more convenient and more consistent

Comments

@avamsi
Copy link
Collaborator

avamsi commented Jan 15, 2023

Current behavior --

o 3728442fe4f3 martinvonz@google.com 1 day ago main* 31ad0cd2ed36
| formatter: reset color around newlines
o a48caf2e5753 avamsi07@gmail.com 15 hours ago push-4fe214e50ae1 HEAD@git 1
5a08e021ff9
| let branches and remote_branches revset functions take needles as argumen
ts
@ f5bdc08c86aa avamsi07@gmail.com 15 hours ago c3d77fa6a56f
  (no description set)

Desired behavior (note the graph is not "broken" in this example) --

o 3728442fe4f3 martinvonz@google.com 1 day ago main* 31ad0cd2ed36
| formatter: reset color around newlines
o a48caf2e5753 avamsi07@gmail.com 15 hours ago push-4fe214e50ae1 HEAD@git
| 15a08e021ff9
| let branches and remote_branches revset functions take needles as
| arguments
@ f5bdc08c86aa avamsi07@gmail.com 15 hours ago c3d77fa6a56f
  (no description set)

I picked a straightforward example but this makes even more sense for deeply nested commits (and more relevant if/when log templates are documented and people go for more compact representations in a single line e.t.c.)

All that said, this may not be desired by everyone, so maybe this could be a default-false flag to jj log.

@avamsi avamsi added the polish🪒🐃 Make existing features more convenient and more consistent label Jan 15, 2023
@martinvonz
Copy link
Owner

For reference, Mercurial does this (as you probably know). It has termwidth and graphwidth keywords and fill() function for doing it in a template.

yuja added a commit that referenced this issue Mar 4, 2023
Template functions like indent() or fill() need to manipulate labeled
output. Since indent() is line oriented, it could be implemented as a
post-processing filter. OTOH, fill()/wrap() inserts additional "\n"s. If we
do that as a post process, colorized text could be split into multiple lines,
and would mess up graph log output. By using FormatRecorder, we can apply
text formatting in between labels.

I thought we could disallow text wrapping of labeled template fragments, but
the example in #1043 suggests that we do want to wrap(whole_template_output)
rather than simple description.wrap().
yuja added a commit that referenced this issue Mar 10, 2023
The parameter order follows indent()/label() functions, but this might be
a bad idea because fill() is more likely to have optional parameters. We can
instead add template.fill(width) method as well as .indent(prefix). If we take
this approach, we'll probably need to add string.fill()/indent() methods,
and/or implicit cast at method resolution. The good thing about the method
syntax is that we can add string.refill(), etc. for free, without inventing
generic labeled template functions.

For #1043, I think it's better to add a config like ui.log-word-wrap = true.
We could add term_width/graph_width keywords to the templater, but the
implementation would be more complicated, and is difficult to use for the
basic use case. Unlike Mercurial, our templater doesn't have a context map
to override the graph_width stub.
@yuja yuja closed this as completed in 904e9c5 Mar 11, 2023
@avamsi
Copy link
Collaborator Author

avamsi commented Mar 11, 2023

Woot, thanks @yuja!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish🪒🐃 Make existing features more convenient and more consistent
Projects
None yet
Development

No branches or pull requests

2 participants