On Fri, Oct 4, 2019 at 4:49 AM Phillip Wood <phillip.wood...@gmail.com> wrote:
>
> Hi Junio
>
> On 03/10/2019 06:04, Junio C Hamano wrote:
> > Here are the topics that have been cooking.  Commits prefixed with
> > '-' are only in 'pu' (proposed updates) while commits prefixed with
> > '+' are in 'next'.  The ones marked with '.' do not appear in any of
> > the integration branches, but I am still holding onto them.
> > [...]
> >
> >
> > * pw/rebase-i-show-HEAD-to-reword (2019-08-19) 3 commits
> >   - sequencer: simplify root commit creation
> >   - rebase -i: check for updated todo after squash and reword
> >   - rebase -i: always update HEAD before rewording
> >   (this branch is used by ra/rebase-i-more-options.)
> >
> >   "git rebase -i" showed a wrong HEAD while "reword" open the editor.
> >
> >   Will merge to 'next'.
>
> That's great, thanks
>
> >
> > * ra/rebase-i-more-options (2019-09-09) 6 commits
> >   - rebase: add --reset-author-date
> >   - rebase -i: support --ignore-date
> >   - sequencer: rename amend_author to author_to_rename
> >   - rebase -i: support --committer-date-is-author-date
> >   - sequencer: allow callers of read_author_script() to ignore fields
> >   - rebase -i: add --ignore-whitespace flag
> >   (this branch uses pw/rebase-i-show-HEAD-to-reword.)
> >
> >   "git rebase -i" learned a few options that are known by "git
> >   rebase" proper.
> >
> >   Is this ready for 'next'.
>
> Nearly, but not quite I think cf [1]. Also I'm still not convinced that
> having different behaviors for --ignore-whitespace depending on the
> backend is going to be helpful but maybe they are close enough not to
> matter too much in practice [2].

Sorry I should have chimed in sooner; I can speak to the second point.
I would say that in practice it doesn't matter a lot; in most cases
the two overlap.  Both am's --ignore-whitespace and merge's
-Xignore-space-change are buggy (in different ways) and should be
fixed, but I'd consider them both to be buggy in edge cases.  I
recommended earlier this summer that Rohit submit the patches without
first attempting to fix apply or xdiff, and kept in my TODO list
that'd I'd go in and fix xdiff later if Rohit didn't have extra time
for it.  I did a little digging back then to find out the differences
and suggested some text to use to explain them and to argue that they
shouldn't block this feature:

"""
am's --ignore-space-change (an alias for am's --ignore-whitespace; see
git-apply's description of those two flags) not only share the same
name with diff's --ignore-space-change and merge's
-Xignore-space-change, but the similarity in naming appears to have
been intentional with am's --ignore-space-change and merge's
-Xignore-space-change being designed to have the same functionality
(see e.g. the commit messages for f008cef4abb2 ("Merge branch
'jc/apply-ignore-whitespace'", 2014-06-03) and 4e5dd044c62f
("merge-recursive: options to ignore whitespace changes",
2010-08-26)).  For the most part, these options do provide the same
behavior.  However, there are some edge cases where both apply's
--ignore-space-change and merge's -Xignore-space-change fall short of
optimal behavior, and in different ways.  In particular,
--ignore-space-change for apply will handle whitespace changes in the
context region but not in the region the other side modified, and
-Xignore-space-change will delete whitespace changes even when the
other side had no changes (thus treating both sides as unmodified).
Fixing these differences in edge cases is left for future work; this
patch simply wires interactive rebase to also understand
--ignore-whitespace by translating it to -Xignore-space-change.
"""

I've got another email with even more detail if folks need it.

Reply via email to