Hi Elijah & Stolee, On Sat, 29 Apr 2023, Elijah Newren wrote: > On Mon, Apr 24, 2023 at 8:23 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > > > On 4/22/2023 9:18 PM, Elijah Newren wrote: > > > On Thu, Apr 20, 2023 at 6:44 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > >> > > >> On 4/20/2023 12:53 AM, Elijah Newren wrote: > > >>> On Tue, Apr 18, 2023 at 6:10 AM Derrick Stolee <derrickstolee@xxxxxxxxxx> wrote: > > > > >> 3. (Ordering options) Modifications to how those commits are ordered, > > >> such as --topo-order, --date-order, and --reverse. These seem to > > >> be overridden by git-replay (although, --reverse probably causes > > >> some confusion right now). > > > > > > Yep, intentionally overridden. > > > > > > Could you elaborate on what you mean by --reverse causing confusion? > > > > It's very unlikely that a list of patches will successfully apply > > when applied in the reverse order. If we allow the argument, then > > someone will try it thinking they can flip their commits. Then they > > might be surprised when there are a bunch of conflicts on every > > commit. > > > > Basically, I'm not super thrilled about exposing options that are > > unlikely to be valuable to users and instead are more likely to cause > > confusion due to changes that won't successfully apply. > > Oh, I got thrown by the "right now" portion of your comment; I > couldn't see how time or future changes would affect anything to make > it less (or more) confusing for users. > > Quick clarification, though: while you correctly point out the type of > confusion the user would experience without my overriding, my > overriding of rev.reverse (after setup_revisions() returns, not before > it is called) precludes that experience. The override means none of > the above happens, and they would instead just wonder why their option > is being ignored. FWIW here is my view on the matter: `git replay`, at least in its current incarnation, is a really low-level tool. As such, I actually do not want to worry much about protecting users from nonsensical invocations. In that light, I would like to see that code rejecting all revision options except `--diff-algorithm` be dropped. Should we ever decide to add a non-low-level mode to `git replay`, we can easily add some user-friendly sanity check of the options then, and only for that non-low-level code. For now, I feel that it's just complicating things, and `git replay` is in the experimental phase anyway. And further, I would even like to see that `--reverse` override go, and turn it into `revs.reverse = !revs.reverse` instead. (And yes, I can easily think of instances where I would have wanted to reverse a series of patches...). Ciao, Johannes
