git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood@talktalk.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Philip Oakley <philipoakley@iee.org>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
Date: Mon, 21 Aug 2017 11:32:48 +0100	[thread overview]
Message-ID: <a3b7af29-8b3a-5253-21da-957920212a6e@talktalk.net> (raw)
In-Reply-To: <xmqqpocloqcp.fsf@gitster.mtv.corp.google.com>

On 27/07/17 16:24, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>>> On 26/07/17 23:12, Junio C Hamano wrote:
>>>> I think
>>>> you are already 80% there without adding a yet another option,...
>>> ...
>>> I'm interested in the 20% as it's about 100% of my rebase conflicts.
> 
> OK, then at least a fixed --rerere-autoupdate would hopefully limit
> the scope of the additional 20%; I'd suspect that a new option would
> also internally turn on --rerere-autoupdate, so that the remaining
> changes you would see upon --continue would be limited to what the
> user had to manually resolve (and edit without having textual conflict,
> aka evil merge to resolve semantic conflicts).

I can see the logic in that but I was imagining (and the patches
implement) that the --autostage option would be passed to 'rebase
--continue' not when the rebase was started by which time it is too late
to turn on --rerere-autoupdate for the current conflicts. I prefer
having to pass --autostage with --continue so that it is a concious
decision by the user to stage unstaged changes when they continue rather
than rebase just doing it each time it continues.

> I am *not* opposed to an option to tell the command to blindly take
> such remaining changes, as long as it stays optional---the use of
> the option can then be taken as a strong signal that the user is OK
> with the local changes in the working tree, even if the user may not
> have marked them as resolved with "git add".

I agree it definitely needs to be optional (possibly with a config
option to have it on by default but I'm not wedded to that as it is easy
to set up a git or shell alias)

What is the best way forward on this? The patches I posted add a
'--autostage' option to be passed with '--continue' which means that
staging all the unstaged changes remains optional but does not allow
--rerere-autoupdate to be automatically enabled. I'm not sure about the
check for merge markers, as it is implemented it also checks for
whitespace errors which is really the domain of the pre-commit hook. If
we go for an explicit --autostage without the config key to make it on
by default then maybe it is OK to drop the check, but it keeping it
could be a useful safety measure. I don't think it would be too much
work the change diff to allow '--check=merge-markers' as internally the
whitespace and marker checks are implemented separately.

Best Wishes

Phillip

>>>> And from the
>>>> workflow point of view, encouraging them to "git add" their manual
>>>> resolution after they are satisified with their changes by not doing
>>>> "git add" blindly for all changes, like your --autostage" does, is
>>>> probably a good thing.
>>
>> Git allows 'git commit -a' to complete a conflicted merge which I
>> think is much the same thing as I'm proposing....
> 
> "-a" is a strong enough sign that the user is OK with all the paths;
> "git commit" without an option does not.  So it is OK for a new
> option (perhaps "--all-autoupdate", which does more than the
> existing "--rerere-autoupdate") to become the signal that the user
> is OK with all the local changes.
> 
> This is a tangent, but concluding a merge with "commit -a" (or "add
> -u && commit -a") has always been discouraged among Git expert
> users, and it will stay to be so.  If you search the list archive,
> you would find a good explanation by Linus on this, but a short
> version is that this is because it is normal to start a merge in a
> dirty working tree where the user has local changes that the user
> knows will not interfere with the merge.  
> 
> Because "rebase" refuses to work in a dirty working tree, the
> analogy with "merge" does not quite hold.  Doing "add -u" before
> telling it to "--continue" would be much safer.
> 
> Thanks.
> 
> 
> 


  reply	other threads:[~2017-08-21 10:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 2/5] rebase -i: improve --continue --autostage Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 3/5] Unify rebase amend message when HEAD has changed Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 4/5] Add tests for rebase --continue --autostage Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 5/5] Add rebase.continue.autostage config setting Phillip Wood
2017-07-26 18:21 ` [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Junio C Hamano
2017-07-26 21:56   ` Junio C Hamano
2017-07-26 22:12     ` Junio C Hamano
2017-07-27 10:36       ` Phillip Wood
2017-07-27 13:25         ` Phillip Wood
2017-07-27 15:24           ` Junio C Hamano
2017-08-21 10:32             ` Phillip Wood [this message]
2017-08-21 22:41               ` Junio C Hamano
2017-08-22 10:54                 ` Phillip Wood
2017-08-22 15:54                   ` Junio C Hamano
2017-08-24 13:25                     ` Phillip Wood
2017-08-24 16:46                       ` Junio C Hamano
2017-08-25 11:54                         ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a3b7af29-8b3a-5253-21da-957920212a6e@talktalk.net \
    --to=phillip.wood@talktalk.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.org \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).