diffedit: start the builtin ui with all boxes checked

[v-gb]

(fixes #5390)

Currently, there is a blocker: when quitting the ui with 'q' after making no changes, you get prompted with 'you have modified N files, are you sure?', which seems like a bug in the scm_record library.

Separately, I tested this manually but I'm unsure whether and how I should add automated tests. The problem is that this change only affects the builtin UI, which I don't know how to test automatically. If I make external tools support this new InitialSelection parameter, I could test that diffedit receives InitialSelection::All, but not the bulk of the change.

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

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1209241374115279

[arxanas]

Thanks for implementing this (and filing the bug upstream on scm-record)!

Re testing:

• I guess it would be good to add a unit test to mod tests to ensure that InitialSelection::All is respected for all kinds of sections we know how to render, and ensure we have some coverage for that code path.
• I don't see a good way of adding a test that confirms that the InitialSelection is threaded all the way down to the appropriate diff editor usage sites.
  • IIRC we don't have any TUI tests at present.
  • You could also reasonably argue that it's a specification issue rather than an implementation issue, so such a test may be low-value compared to implementation effort.
  • Having the enum should reduce likelihood of type confusion with other booleans.

[arxanas]

comment (non-blocking): We could consider if we want to bundle the "configuration" kind of arguments here into a single struct for organizational reasons, but I don't feel strongly.

[v-gb]

Thanks! I'll see about adding some unit tests.

[yuja]

Maybe better to always open builtin editor with everything checked?

It's weird that initial_selection applies only to the builtin editor. External editor opens with the right content (i.e. everything selected?)

[arxanas]

I don't have a strong opinion myself, but the per-command difference was discussed here: #5390 (comment)

It seems to make sense to me that we would want e.g. split to start with nothing but diffedit to start with everything. Maybe it's more of a limitation with external diff editors? Is there a way to indicate to an external diff editor "here is a set of changes, but don't apply any of them by default"?

[martinvonz]

I don't think there's a way of telling external 2-way diff editors that. But we can tell external 3-way diff editors that by setting the middle state to be either the same as the left (for split) or the same as the right (for diffedit).

By the way, another aspect is what the effect of starting the diff editor and immediately quitting should do. One could argue that it should have the same effect as not even starting it. split and diffedit are always interactive (unless you pass paths to split), but squash and restore can be non-interactive. Is it important to us that jj squash has the same effect as running jj squash -i and immediately closing the diff editor? I'm fine with that inconsistency.

[yuja]

another aspect is what the effect of starting the diff editor and immediately quitting should do.

Good point. I slightly prefer that -i (with noop tool) works in the same way as non-interactive edit.

[v-gb]

I haven't used external diff tools, so trying meld and meld-3 just now, they apparently start with everything selected (whereas the builtin ui starts with nothing selected). On the face of it, the inconsistency is a bit weird.

As Martin says, for 3-way diff editors, you could change this by choosing the middle file. For 2-way diff editors, you could change the default by swapping left and right, although that'd possibly be too confusing to use.

If we make the diff tools consistent, the most disrupting change would probably be jj commit -i. Thinking about it, I don't see a strong reason to prefer starting from the empty diff vs the working copy diff. It seems to be a question of habit.
In this case, git commit --interactive doesn't really set expectations one way or the other (not sure it's used much, as it's quite strange). With magit, you create commits from the empty diff, but it's something you do before calling the git commit equivalent, so it's like using git add -p + git commit, it doesn't seem quite comparable.

No clear conclusion.