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

help text for rebase is inconsistent #5218

Open
joyously opened this issue Dec 31, 2024 · 9 comments
Open

help text for rebase is inconsistent #5218

joyously opened this issue Dec 31, 2024 · 9 comments
Labels
documentation Improvements or additions to documentation

Comments

@joyously
Copy link

Description

My reading of the jj help rebase text is perhaps tainted by seeing -r used on other commands as plural, so I equate it with "revset". I got confused by the first paragraph saying that -s is for "a revision and its descendants" followed immediately by -r for "a single commit". They seemed backward to me, so the examples were a bit much especially since they take up enough room that I can't fit the first paragraph explanation on the same screen as the option descriptions. In addition, the example graphs have visible Markdown taking up space.
The option description says that -s is for source and -r is for revisions (plural).

This is one explanation where using both "commit" and "revision" is very confusing.
The help needs to say a little more about rebase than simply "move revisions to different parents", to give at least a hint of what changes and/or what consequences or limits there are since novice users need that.

Steps to Reproduce the Problem

  1. jj help rebase
  2. compare first paragraph to option definitions

Expected Behavior

I expect option letters to make sense.

Actual Behavior

Option letters represent renamed concepts ( -b for branch ), no match for the letter ( -s for "a revision and its descendants" ), and different plurality ( -r is "single commit" versus "--revisions" )

Specifications

  • Platform: Ubuntu Studio 24.04
  • Version: jj 0.23.0
@PhilipMetzger PhilipMetzger added the documentation Improvements or additions to documentation label Jan 1, 2025
@arxanas
Copy link
Contributor

arxanas commented Jan 1, 2025

Link for reference: https://jj-vcs.github.io/jj/v0.24.0/cli-reference/#jj-rebase

My reading of the jj help rebase text is perhaps tainted by seeing -r used on other commands as plural, so I equate it with "revset".

I think the documentation is misleading and I believe -r can take an arbitrary revset, not just a single revision, as you expected.

  • This is useful in case you want to "splice out" a set of commits and rebase it.
  • It's corroborated later in the docs when it refers to "specified revisions".

I got confused by the first paragraph saying that -s is for "a revision and its descendants" followed immediately by -r for "a single commit". They seemed backward to me, so the examples were a bit much especially since they take up enough room that I can't fit the first paragraph explanation on the same screen as the option descriptions. In addition, the example graphs have visible Markdown taking up space.

[historical note] I suspect the -b and -s options were taken directly from Mercurial, and -r may not have existed at first, hence the documentation might have been written in the "wrong" order.

I agree that, from a conceptual perspective, it probably makes sense to discuss the -r case first, and define -s and -b flags in terms of that behavior.

I personally don't like -b/--branch and -s/--source as names that much.

  • I don't have better naming ideas.
  • Maybe it would even be possible to remove them altogether if we had better revset syntax.
    • How much do we benefit from being able to write -s 'foo()' instead of `-r 'foo()::'?
    • It seems like mostly a matter of syntax and operator precedence. If we had a "pipeline" or postfix function application operator, then perhaps we could write complicated_expr() |> :: or something like that, without having to fiddle with the syntax too much.

[tangential] I personally have found that the default of -b @ is usually not what I want, since it oftentimes intersects immutable commits. That concept also came into being after -b was created, I believe.

This is one explanation where using both "commit" and "revision" is very confusing.

Agreed. We should definitely be consistent. I think "revision" is a better term to use everywhere.

The help needs to say a little more about rebase than simply "move revisions to different parents", to give at least a hint of what changes and/or what consequences or limits there are since novice users need that.

Agreed. I see now that it actually doesn't mention anything about reapplying patches, so from the specification in the docs here, one could be misled into thinking that it only modifies the graph structure without changing any of the snapshots/trees.

@jennings
Copy link
Contributor

jennings commented Jan 4, 2025

  • How much do we benefit from being able to write -s 'foo()' instead of `-r 'foo()::'?

jj fix only accepts -s and I find that to be a helpful reminder that it will always operate on descendants too. I would have taken longer to memorize what that means if I didn’t already have the knowledge from jj rebase.

[tangential] I personally have found that the default of -b @ is usually not what I want, since it oftentimes intersects immutable commits. That concept also came into being after -b was created, I believe.

I use the default all the time, we do GitHub flow so I habitually run jj rebase -d main to get my feature branches up to date.

@martinvonz
Copy link
Member

no match for the letter ( -s for "a revision and its descendants" )

I agree, and I've considered --descendants in the past, but the short form would conflict with --destination.

@martinvonz
Copy link
Member

I would say that #5400 closes this issue. There's still the markdown issue but that's a generic problem (we could repurpose this issue to be about that, or we could file a new issue about it). What do you think, @joyously?

@joyously
Copy link
Author

I think it would be nice to use a word that starts with s in the help. Is it a slice or a splice or a source or a set or a stack? Or change the s to something more memorable like twig or cousin or dependency.

Also, I didn't see any new description of what the rebase is actually doing besides rearranging the graph. If that's all it does, please say so. Otherwise, say what it does.

@martinvonz
Copy link
Member

I think it would be nice to use a word that starts with s in the help. Is it a slice or a splice or a source or a set or a stack? Or change the s to something more memorable like twig or cousin or dependency.

It comes from Mercurial. The long form is --source. If we were to come up with a name from scratch, I might call it --from, and I might rename the current --destination to --onto. I don't know if others like those names or if the have better names. Even if we think of names that people generally like better than the current names, we have to weigh the cost of changing the name.

Also, I didn't see any new description of what the rebase is actually doing besides rearranging the graph. If that's all it does, please say so. Otherwise, say what it does.

I added a sentence about it in #5400. I think just "rearranging the graph" is too vague. I tried to clarify a bit about how it does it.

@joyously
Copy link
Author

It comes from Mercurial. The long form is --source.

Don't take this the wrong way, but it doesn't help users to explain in the issue comments. The help text is what they are going to read, and most of them have not used Mercurial (including me). The option name should make sense.

The only thing I could see that you added was a sentence that repeated the short summary and said "while preserving the changes (diff) in the revisions". This does help some, but I was expecting to find something about immutable and remote versus local and change versus revision...just a brief caution about what the command affects.

Perhaps this help text is so difficult to read because the Usage is at the bottom, defining the options after the explanation with examples. Anything you add to the explanation is a repeat of the option definition, so maybe the two should be more combined instead. Also, the sentence defining the -b, -s, -r options shouldn't repeat the verb.

Despite the -b option being the default and the easiest to understand because it stands for "branch", jj doesn't have branches, so it's a little confusing referring to bookmarks everywhere else, but a branch here in a graph context.

@martinvonz
Copy link
Member

Don't take this the wrong way, but it doesn't help users to explain in the issue comments.

It wasn't meant to help users :) It was meant as input to the discussion here about changing the name of the flag and/or the description text.

The option name should make sense.

Right, that's why I proposed two names (--onto and --from) I think make more sense.

This does help some, but I was expecting to find something about immutable and remote versus local and change versus revision...just a brief caution about what the command affects.

I feel like most of that belongs in a more central place because it's not specific to the rebase command.

I see that have not documented what immutable commits are in the glossary.

I think we should add it there. I don't think we've discussed remote vs local revisions anywhere. That might deserve its own section under Concept.

Despite the -b option being the default and the easiest to understand because it stands for "branch", jj doesn't have branches, so it's a little confusing referring to bookmarks everywhere else, but a branch here in a graph context.

I actually thought one of the good things about renaming "branches" to "bookmarks" was that we freed up "branch" to always refer to topological branches (well, except that we also talk about inteterop with Git branches in many places).

@joyously
Copy link
Author

I probably have more knowledge of jj than most novices would have, and I find this help text almost useless for determining if I want to use this command.
If the help could answer the questions used in journalism (What, When, Why, How), it would be adequate for several levels of experience. There can be links to more in-depth explanations. The order of them is debatable, because of different learning styles, but talking about using options before defining what all the options are throws me off. Explaining the options using the words that inspired the option still seems best.
Perhaps that's the crux of the problem with rebase. The word isn't recognized by the spell checker in this browser, and it isn't really defined in the help either (at least not using the word "base"). If you've been exposed to VCS at all, the word has a taint of complexity and "evil" that needs to be dispelled by explaining what is happening. You added that the diff is preserved, but did it change because of the move (is it compared to a different base)?

One other thing I noticed is that there can be multiple -s and -r, but can you mix -s and -b and -r ? If they really are just different "sources", maybe it should just be -r with a branch option or a from option that indicates how to treat the descendants.

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

No branches or pull requests

5 participants