Re: [PATCH] Warnings before rebasing -i published history
- From
- konglu@minatec.inpg.fr <konglu@minatec.inpg.fr>
- Date
- Jun 8, 2012, 07:32 UTC
- Message-ID
- <20120608093257.Horde.eeddeHwdC4BP0aqpIrqTaVA@webmail.minatec.grenoble-inp.fr>
- In-Reply-To
- <7vwr3ieo0i.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> a écrit :
Show 19 quoted lines
>> + 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 > > What's inside $() looks like it wants to say something like > > sed -e "/ $sha1 /s/$/ [Published]/" "$1" > > but it has a few fishy double-quotes that makes it unclear why $sha1 > wants to be outside the quotes. > > Why does it need 'printf "%s" $()' in the first place? Wouldn't > > sed ... >"$1" > sufficient?
The fact is that the input of sed is "$1" itself. An redirecting the output of sed to the same input doesn't seem to work.
Show 7 quoted lines
> The logic is merely _guessing_ that the commit could have been > published, no? The particular remote repository the test happens to > find may not be for consumption by other people. > > I am afraid that doing this would send users a wrong message that is > unnecessarily alarming, especially the marker says "Published" as if > it were a confirmed fact.
True. It would be annoying for people that rewrite consciously published history. From this point of view, a better idea would be to show the remote branch name, as Matthieu suggested, so that the user could know which branch is involved.