jj-vcs
FR: show `(has changes)` if the commit is non-empty but doesn't have a description
Updated proposal
Thanks for the feedback. The current proposal, from this comment is:
If a commit is non-empty but doesn't have a description, show (has changes).
Original proposal
Is your feature request related to a problem? Please describe. When looking at log output, I'd like the "exceptional" or "actionable" cases to stand out a bit. We currently do this in the CLI to some extent.
| empty? | has description? | text |
|---|---|---|
| false | false | (no description set) [in yellow] |
| false | true | |
| true | false | (empty) (no description set) [in green] |
| true | true | (empty) |
This means that to know if a commit has changes I need to look for the absence of '(empty)'. This is a little awkward.
Describe the solution you'd like
| empty? | has description? | text |
|---|---|---|
| false | false | (has changes) (no description set) [in yellow. maybe?] |
| false | true | |
| true | false | (no description set) [in green] |
| true | true | (empty) |
That has three changes from the current behavior:
- "(has changes)" is called out explicitly iff there's no description, because a commit with a description and changes is an expected state with no action required.
- Remove "(empty)" when there's no description set, because that's not an edge case, it's standard when someone runs
jj new. Showing "(empty)" implies that the commit being empty is "weird" in some way, or that the user needs to take an action. - Change the color of (empty) when there is a description. It shouldn't be red or anything "scary", but I think the same yellow color as in (no description set) would be fine? I'm trying to draw attention to the fact that a commit is empty, but has a description (since it's a less common workflow to have that), without making it stand out to an awkward extent for users that do regularly encounter that state in their workflow.
Describe alternatives you've considered
- We could leave it as-is, but I think that it can be improved.
- We could (perhaps only temporarily, while people are adjusting their habits to look for "(has changes)" instead of the lack of "(empty)")) keep the third row as "(empty) (no description set)", and decide later if we want to remove the "(empty)" or not
Additional context Honestly I'd love it if we changed "(no description set)", but that can be a separate FR if people are interested :P
I found the old title a bit misleading (first, I thought you wanted to print it in inverted text, then I thought you wanted the exact opposite of current behavior) I tried to improve it, with IMO mixed success; feel free to adjust it further.
If we go with this proposal, I also think the (empty) in the last line of your table should probably be yellow.
I can't make up my mind to what degree I agree with it; maybe it's worth trying out in my personal config and seeing how it goes.
I'm curious how you'd want to change (no description set), that may or may not be independent from this proposal.
"(has changes)" is called out explicitly
This is probably an improvement.
Remove "(empty)" when there's no description set, because that's not an edge case, it's standard when someone runs jj new. Showing "(empty)" implies that the commit being empty is "weird" in some way, or that the user needs to take an action.
I heartily disagree with this one. It doesn't imply anything of the sort, it just accurately tells me the state of the change, and I would hate for it to be removed. It could make sense to remove the (no description set) because that's a subset of being empty, but it feels nicer to be explicit.
[My apologies for the long reply here]
Full proposal
I think I didn't include as much context as I should have; I didn't include all of the details for what I think of as an ideal end state, because I thought breaking it down would make it easier to have good discussions and reach consensus. That was probably going a bit too far, so my desired state:
Today: Proposed:
@ llll @ llll
│ (no description set) │ (working copy) (has changes)
│ ○ mmmm │ ○ mmmm
│─╯ (empty) (no description set) │─╯ (empty) (no description)
│ ○ nnnn │ ○ nnnn
│─╯ (no description set) │─╯ (has changes) (no description)
│ ○ oooo │ ○ oooo
│─╯ (empty) lorem ipsum # in green │─╯ (empty) lorem ipsum # NOT in green
│ ○ pppp │ ○ pppp
│─╯ dolor sit amet │─╯ dolor sit amet
(yes, mmmm is unlikely, but it's possible).
The graphs have the following differences:
(working copy)shows up as a thing that's explicitly calling out the working copy. In my local CLI I have this as blue, but color is tricky for us (I'll elaborate below)- If a commit is has changes (non-empty) but doesn't have a description: show
(has changes) - If a commit doesn't have changes and ISN'T the working copy: show
(empty) - If a commit doesn't have a description and ISN'T the working copy: show
(no description)
That's slightly different from what I asked for in the first message in this FR; I originally suggested not showing (empty) in any situation, but I looked at the proposed graph without it for mmmm and it felt weird. I still think I'd prefer not having it if it's the working copy commit, but if others disagree, I think it's acceptable to have it (showing (working copy) (empty) doesn't risk immediately breaking the fiction that the working copy commit is some virtual thing [see below for where I'm going with that])
This probably needs some additional nuance/consideration. Maybe we want to show (no description) if the commit has a child, even if it's the working copy commit, since that's "weird" and clearly the working copy commit isn't a "virtual" commit and we don't need to support the user in thinking it is.
This issue
If we want to discuss the full proposal in this issue, should I modify the original text? Open a new issue? I'm not super familiar with GitHub etiquette.
Similarly, If we want to keep this proposal more focused, I'd rewrite the proposal to be "Show (has changes) if the commit has changes and doesn't have a description". Should I modify the original text?
Which course of action would people prefer? :)
Support GUI-first / GUI-only users
Backing up a bit, my reasoning here was that we have an internal IDE extension at $dayjob and we're expecting thousands of users to start using jj via this extension and that they'll never even touch the CLI or have been forced to read any documentation. This has several consequences:
- users see a commit graph like they have with our Mercurial-based solution, and think they know how to use the system. Essentially 100% of users have been confused by the working copy commit when they start using the system this way, and they all gravitate to the "Edit" (vs. "New + Squash") workflow, and I think that's likely going to go poorly for them (and my team who has to support them)
- Differentiation via color is MUCH harder due to the IDE's support for themes, and these themes often having primitives that aren't specific enough (there might be a "text color" variable (let's say "bright gray" in a dark theme) but there might not be a "everything is good" (~green), "there's some concerns" (~yellow), "there's an error" (~red) text color) or they might not be composable (example: even if there is a "everything is good" text color, if you change the background color because the commit is highlighted/selected/hovering over it, you don't have sufficient contrast to be readable)
That #2 means that we need to highlight the working copy commit in some way that doesn't rely on color. We added a chip (a rounded rectangle) with a background color and text color that says "working copy" in the graph in this extension. I'd like to do the same on the CLI as well, but it's less important there. I've still included it in the proposal above.
So with all of that in mind, my goal had been to simplify the graph as much as possible, so that we supported / sort of encouraged users to think of the current commit, if it doesn't have a description, as a "virtual" commit that doesn't really exist. JJ will actually delete it if you move away from it, so there's even a little bit of supporting evidence for this. The thinking being that if people want to treat this like Git's "staging area", that's fine! They don't ever need to know that it's a real commit under the hood. If they realize it's a real commit? That's great, now they can interact with it like every other commit. Removing text like (empty) was mostly because we couldn't differentiate via color whether that was "empty but that's pretty normal" and "empty but that's kinda weird".
And finally, I was trying to come up with something that worked for both CLI and GUI, but I was definitely focusing more on the GUI aspect, so I think I skipped providing some details that were important.
New+Squash vs. Edit
When I proposed this internally at $dayjob, I got some pushback, and I expect everyone reading this has already started planning their responses ;) But please hold off for a second. I know it's not a virtual commit. You know it's not a virtual commit. Anyone who's read the documentation knows it's not a virtual commit. Anyone who's used the CLI for more than 2 minutes knows it's not a virtual commit.
BUT: if your first exposure to the system is via a graphical interface such as an IDE, and it supports double clicking to run jj new (unless the commit has no children and no description, then it does jj edit), drag-and-drop to rebase things, and there's a button labeled commit that (a) gives the current commit a description and (b) creates a new commit (i.e. it's jj commit)... there's nothing that forces a user to learn that the working copy commit is a full fledged commit instead of just a fiction in the GUI. If they want to think of it as a virtual commit that's fine.
Yes, this is primarily me trying to guide users towards New + Squash instead of Edit. I think Edit is wonderful... if you know what it does. New users -- especially ones that opened their IDE, saw a commit graph, thought to themselves It's a commit graph! I know this! and didn't search out any documentation on the recommended workflow -- won't understand the nuances and pitfalls of the Edit workflow. It's been difficult to convince people internally at $dayjob that jj edit has no equivalent in Git or Mercurial.
The most frequent issue we've had at $dayjob is people "accidentally" (by running jj describe and not immediately running jj new) or "intentionally but not informed" falling into the Edit workflow, and then needing to split out the changes. Yes, jj can do that pretty easily. But most of those people have eventually found that they prefer to default to the new+squash workflow. There was a post in our internal chat room about someone who recently changed their mind and switched from Edit to New+Squash, because it fit their workflow better.
So maybe this is a long-winded way of saying "Maybe we should rename jj new to be jj edit, and jj edit to be jj edit --directly". I'm not actually serious. Unless... No, no, you're right, that's crazy. Or is it? Yeah, it is, ok.
(OK, now feel free to tell me why I'm wrong ;))
Why change "(no description set)"?
I'm curious how you'd want to change (no description set), that may or may not be independent from this proposal.
Multiple ways...
- as you can see above I think it'd be cleanest if we didn't show anything if this was the working copy commit. Reasoning: if users want to think of it as a virtual commit, well, virtual commits wouldn't normally be something that you'd give a description, so seeing anything about a description kind of breaks that illusion a little too early (I'm thinking of the first-hour-with-jj-in-an-IDE-extension users). I think having commits that are empty + no description + NOT working-copy not have ANY text would look weird, so I was going to show it if it wasn't the working copy commit.
- "(no description set)" may imply to the user that the next step is
jj describeor equivalent action in the GUI. This implicitly guides users to an edit workflow, even if only temporarily. If we're going to show the text, I think(no description)is cleaner (shorter, less likely to imply that one should set the description as it's just a statement that it doesn't have one).
It is in jj's README that the working copy is a commit and that is part of what unifies the interface. It seems rather odd to be suggesting changes to special case the working copy, just so your colleagues can have some fiction about it.
IMHO, an editor is not where you should be doing version control.
If/when jj becomes wildly popular, we should not assume that people will read the README, or that they'll be using the CLI at all. There are multiple GUIs available already, including vscode extensions. Yeah, today they need to be installed manually, which is where most people would likely read the documentation and learn about this concept, but that's not going to be true forever. Meanwhile, users and people providing support are having a bad time, and I think a few small tweaks like this can really help significantly.
we should not assume that people will read the README, or that they'll be using the CLI at all.
I agree. What I was referring to is that having the working copy as a commit is a foundational concept of jj, so it doesn't make much sense to promote a different philosophy. There are several issues here in GitHub where a maintainer has said "we don't want to make a special case, but treat all commits the same". That's why I don't think the log should special case the working copy, which already has a special label.
What exactly are "users and people providing support having a bad time" about? Surely it's not the log colors.
My opinion about using an editor is not part of this discussion (however editors are different from GUIs specifically for version control). But if you are making an interface, make it anyway you want without changing jj.
But if you are making an interface, make it anyway you want without changing
jj.
I don't think implying that I should not even propose changing things in jj is constructive. I legitimately think these proposals improve things on the CLI for all projects, not just our internal repository and not just via our interface.
That's why I don't think the log should special case the working copy, which already has a special label.
The special label is an @ in a graph that has other round shapes in it. It does not stand out at all. The colors for that row are brighter, which helps draw attention to the working copy commit, but it's still easy to get lost in jj log.
What exactly are "users and people providing support having a bad time" about? Surely it's not the log colors.
I mentioned this in my previous post: the "edit" workflow is an advanced workflow that has sharp corners. I think we need to be pragmatic about making the default interface being one that is intuitive to use (even if that intuition is slightly wrong) in a way that has the fewest guns pointed at your foot. I personally don't think anyone should try to use jj edit in the first day of using jj, and yet that's what every single user we've asked at $dayjob tried to do, unless they were already familiar with jj.
I didn't get any advice for how to adjust the proposal, so I'll edit the original message with a link to this comment once I submit it, saying that the proposal has been narrowed in scope a bit.
Proposal
If a commit is non-empty but doesn't have a description, show (has changes).
If we can agree on that, I may create other FRs at some point in the future where we can debate the more contentious aspects of my full vision.
I personally don't think anyone should try to use
jj editin the first day of usingjj, and yet that's what every single user we've asked at $dayjob tried to do
That seems perfectly logical since you said the environment is an IDE. They are in an editor, so they have "edit" in their brain. Would these same folks go for an "edit" alias if they were using Git? I think the interface focuses too much on Git artifacts like commits and a graph. Maybe it should emphasize the size of changes, their descriptions, and the files more.
Would these same folks go for an "edit" alias if they were using Git?
Yes, most likely, if 'checkout' wasn't an available verb, which it isn't with jj.
When addressing comments from code review, or just wanting to amend a commit, our users are used to a workflow of:
- checkout
- make their changes
- amend
- push
GitHub users are used to a workflow of:
- checkout/switch
- make their changes
- commit
- push
If you take away 'checkout', they go looking for something else. If they just read jj help, or if they're just interacting with a GUI, then the options they're going to see are new and edit (and let's be honest: they probably won't see new, or will skip over it).
I mentioned this in my previous post: the "edit" workflow is an advanced workflow that has sharp corners. I think we need to be pragmatic about making the default interface being one that is intuitive to use (even if that intuition is slightly wrong) in a way that has the fewest guns pointed at your foot. I personally don't think anyone should try to use
jj editin the first day of usingjj, and yet that's what every single user we've asked at $dayjob tried to do, unless they were already familiar with jj.
It's a different workflow than my preferred one, not a more (or less) advanced one, and plenty of people who are new to Jujutsu prefer it. Some of whom have been known to argue that it has fewer sharp corners and an easier mental model and should be the main one taught to new people.
I also think everyone should definitely try to use jj edit on their first day, since it's a useful part of the basic command set.
Some of whom have been known to argue that it has fewer sharp corners and an easier mental model and should be the main one taught to new people.
This is pretty surprising to hear, and I'd like to hear more, but let's take that somewhere else (if you have links to existing conversations, feel free to add them here or @ me on discord (I'm 'spectral' on the jj discord)).
Regardless, the current proposal isn't tied to edit vs. new+squash or IDE vs. GUI in any way afaict. Is there any reason to not show (has changes) when the commit is non-empty but has no description?
Is there any reason to not show
(has changes)when the commit is non-empty but has no description?
I think it's more visual noise. Perhaps instead of "no description set" it could be "no description of changes".
I think it's more visual noise.
One way of reducing visual noise would be to remove labels like (empty) that aren't necessary, like in my original proposal ;)
But more seriously, the goal of (has changes) is to make these commits stand out. It's highlighting that the commit is in a state where it needs some further work to be done to it: it needs to be given a description, or it needs to be squashed or amended or something. (no description of changes) does not have the same effect (and it is subtly implying that the expected resolution of that state is to give a description to the changes, when it's likely just as common for users to run jj squash instead).
Some of whom have been known to argue that it has fewer sharp corners and an easier mental model and should be the main one taught to new people.
This is pretty surprising to hear, and I'd like to hear more, but let's take that somewhere else (if you have links to existing conversations, feel free to add them here or @ me on discord (I'm 'spectral' on the jj discord)).
Kyle, just look at this bug we have from Matt https://github.com/jj-vcs/jj/issues/4238. I also think this is a natural thing. Also there's https://github.com/jj-vcs/jj/issues/7046#issuecomment-3104754836.
Aside from that, I think a part of your proposal is actually useful to have since it may improve the UX for newcomers.
