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

jj show -T always generates diff #5036

Open
kj opened this issue Dec 6, 2024 · 12 comments
Open

jj show -T always generates diff #5036

kj opened this issue Dec 6, 2024 · 12 comments

Comments

@kj
Copy link

kj commented Dec 6, 2024

Description

When I use -T with jj show, the diff is always generated and printed after the template output.

Steps to Reproduce the Problem

In a jj repository with changes to the working copy.

jj show -T 'self.empty()'

Expected Behavior

Outputs only true or false.

Actual Behavior

Outputs true or false followed by the entire diff of changes in the working copy.

Specifications

  • Platform: Linux x86_64
  • Version: jj 0.24.0-32d2a85539254e9d96f9819072fa5c6ac70dd1e4
@yuja
Copy link
Contributor

yuja commented Dec 6, 2024

jj show is the command to show metadata plus diffs. Maybe you're looking for jj log --no-graph?

@kj
Copy link
Author

kj commented Dec 6, 2024

I've been using exactly that in the meantime and it does work, although I assumed that jj show when used with a template would not show a diff, just like jj log does not show its full output when used with a template.

jj show feels more natural to me for showing information from a single revision (jj show -T vs jj log --no-graph -r @ -T), otherwise I'm not sure what the value is in jj show accepting a template if it's just going to be drowned out by the diff output?

#3010

@yuja
Copy link
Contributor

yuja commented Dec 6, 2024

Well, -T isn't a flag to suppress diff, and jj show is roughly equivalent to jj log --no-graph -p. Maybe we could add jj show --no-patch, but I don't know if that's useful.

@kj
Copy link
Author

kj commented Dec 6, 2024

If -T doesn't suppress the diff, then what is the use of it on jj show? It does replace (other than the graph) the regular output of jj log, and most CLI tools that provide an option for output formatting behave like that. I feel like I may be misunderstanding something? What was the intent of adding it if not to control output?

@martinvonz
Copy link
Member

@yuja, could we make the diff part of the template? We have support for that these days (thanks to you), so it seems like we should be able to do that.

@thoughtpolice
Copy link
Member

thoughtpolice commented Dec 7, 2024

If -T doesn't suppress the diff, then what is the use of it on jj show? It does replace (other than the graph) the regular output of jj log, and most CLI tools that provide an option for output formatting behave like that. I feel like I may be misunderstanding something? What was the intent of adding it if not to control output?

I think it's a historical thing. In the past, we didn't have the ability to include diffs as part of the template (as Martin said), so for show it was just a way to control the formatting of the metadata/header/front matter, rather than anything else. For example, the default output shows the attached bookmarks, IDs, author, etc, which are all controlled by the templating system.

I suspect nobody has really asked for it before this request, because nobody really wanted anything else.

@yuja
Copy link
Contributor

yuja commented Dec 7, 2024

@yuja, could we make the diff part of the template? We have support for that these days (thanks to you), so it seems like we should be able to do that.

That's doable (ignoring word wrapping issues and CLI flags), but it would be surprising if jj show -T suppressed diffs but jj log -T -p didn't. I would rather remove jj show -T if the existence of -T is confusing.

@kj
Copy link
Author

kj commented Dec 7, 2024

I personally find it confusing as I can't think of any situation I'd want to use a template to change the output of the metadata yet still show the diff. If jj log -T is the intended way to get custom output even for a single revision, and if there's really no use case for -T on jj show, then I wouldn't mind the option being removed from jj show for clarity's sake.

Although it still makes sense to me as a user that jj show -T would be a much more intuitive version of the rather awkward jj log --no-graph -r @ -T when you just want to get some specific information about the working copy (I use it in my shell prompt).

@yuja
Copy link
Contributor

yuja commented Dec 7, 2024

To me, jj show is the command to show me a patch, so I've never considered -T would suppress the diff part. (If we had jj patch <subcommand> namespace, jj show might belong there.)

One possible use case of jj show -T is something like jj show -Temail to render email-compatible patch, but I've never used jj show -T to customize the output.

Another idea is to make jj show not render diffs by default, and add jj show -p flag to do so.

@kj
Copy link
Author

kj commented Dec 7, 2024

Isn't that what jj diff is for? I'm still new to Jujutsu, so apologies if I have some fundamental misunderstanding.

@yuja
Copy link
Contributor

yuja commented Dec 7, 2024

Yeah, kind of. jj diff is a general-purpose tool to show diffs between two revisions. jj show is a command to show detailed information about a revision, and to me, it is commit metadata and diffs within that revision.

@martinvonz
Copy link
Member

I'm fine with removing support for jj show -T.

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

No branches or pull requests

4 participants