Thread (1 message) 1 message, 1 author, 2019-02-20

Antw: Re: [PATCH v2] merge-options.txt: correct wording of --no-commit option

From: Ulrich Windl <hidden>
Date: 2019-02-20 07:29:18

quoted
quoted
Junio C Hamano [off-list ref] schrieb am 19.02.2019 um 20:32 in
Nachricht
[ref]:
Elijah Newren [off-list ref] writes:
quoted
+With ‑‑no‑commit perform the merge and stop just before creating
+a merge commit, to give the user a chance to inspect and further
+tweak the merge result before committing.
++
+Note that fast‑forward updates do not need to create a merge
+commit and therefore there is no way to stop those merges with
+‑‑no‑commit.  Thus, if you want to ensure your branch is not
+changed or updated by the merge command, use ‑‑no‑ff with
+‑‑no‑commit.
While the above is an improvement (so I'll queue it on 'pu' not to
lose sight of it), I find the use of "do not need to" above somewhat
misleading.  It solicits a reaction "ok, we know it does not need
to, but it could prepare to create one to allow us to further muck
with it, no?".

IOW, a fast‑forward by definition does not create a merge by itself,
so there is nowhere to stop during a creation of a merge.  So at
least:

	s/do not need to/do not/
Agree.
It also may be a good idea to consider detecting this case and be a
bit more helpful, perhaps with end‑user experience looking like...

  $ git checkout master^0
  $ git merge ‑‑no‑commit next
  Updating 0d0ac3826a..ee538a81fe
  Fast‑forward
    ...diffstat follows here...
  hint: merge completed without creating a commit.
  hint: if you wanted to prepare for a manually tweaked merge,
  hint: do "git reset ‑‑keep ORIG_HEAD" followed by
  hint: "git merge ‑‑no‑ff ‑‑no‑commit next".

or even

  $ git checkout master^0
  $ git merge ‑‑no‑commit next
  warning: defaulting to ‑‑no‑ff, given a ‑‑no‑commit request
  Automatic merge went well; stopped before committing as requested
  hint: if you'd rather have a fast‑forward without creating a commit,
  hint: do "git reset ‑‑keep next" now.

I do not have a strong preference among three (the third option
being not doing anything), but if pressed, I'd say that the last one
might be the most user‑friendly, even though it feels a bit too
magical and trying to be smarter than its own good.

In any case, the hint for the "recovery" procedure needs to be
carefully written.
Actually I think if the user specified "--no-commit" and the merge turns out
to be fast-forward, the user could be asked whether to continue or not (instead
of undoinf afterwards); maybe when entering a response is not possible (batch
processing) the merge should be aborted due to "--no-commit" not being possible
(well actually there would never be a commit, even without that option). The
problem is that without prior inspection of the tree you cannot know whether
the merge will be fast-forward or not: fast-forward being an optimization (taht
is enabled by default) makes life more complicated here.

Regards,
Ulrich Windl
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help