Re: [PATCH] Warnings before rebasing -i published history

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:

> For now, the code only warns the user; he can't revert back to his
> original state. We agree with everyone that he should be given a
> way to abort the rebase.

He already has: empty the todo-list in the editor and you're done.

> +warn_published () {
> +	cat "$1" | while read -r command sha1 message
> +	do
> +		test -n "$sha1" || break
> +		if test -n "$(git branch -r --contains "$sha1")"
> +		then
> +			printf "%s\n" "$(sed -e "/"$sha1"/ s|$| [Published]|" "$1")" >"$1"
> +		fi
> +	done

Aren't you reading and writing from the same file here? Sounds
dangerous.

This appends [Published] to commits that are reachable from a
remote-tracking branch, but I'm wondering if actually showing the remote
branch name wouldn't be better, e.g.

pick <id> commit one
# Commits above this line appear in origin/master
pick <id> commit two
pick <id> commit three
# Commits above this line appear in origin/next
pick <id> most recent commit

>  test -s "$todo" || echo noop >> "$todo"
>  test -n "$autosquash" && rearrange_squash "$todo"
> +warn_published "$todo"

That should be configurable.

> @@ -869,6 +884,8 @@ cat >> "$todo" << EOF
>  #  f, fixup = like "squash", but discard this commit's log message
>  #  x, exec = run command (the rest of the line) using shell
>  #
> +# Warning: [Published] means that the commit has already been published

It's a pity to show the message unconditionally. If you go this way,
skip the message if you didn't display [Published].

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/