Noe Rubinstein <nrubinstein@xxxxxxxxxxxxxxxxx> writes:
> When doing git rebase -i, removing all actions in the todo list is
> supposed to result in aborting the rebase.
I thought it was meant to be more like "removing all _lines_", and the
grep was a half-assed attempt to ignore lines that are clearly comments.
Checking the size of the insn sheet might be a better change in that
sense, as that would not leave any ambiguity:
has_action () {
test -s "$1"
}
> This patch fixes the bug by changing has_action to grep any line
> containing anything that is not a space nor a #.
First of all, I do not think it is a "fixes the bug". I can buy "makes
things safer by detecting user errors", of course.
More importantly, I do not think you are grepping "any line containing
anything that is not a space nor a hash". You are instead grepping lines
that do not begin with a hash or a whitespace, no?
> has_action () {
> - sane_grep '^[^#]' "$1" >/dev/null
> + sane_grep '^[^#[:space:]]' "$1" >/dev/null
> }
We tend to avoid [:character class:] to accomodate older implementations
of grep.
We earlier asked "do we have any line that begins with a character that is
not a hash '#'?" but now we say "do we have any line that begins with a
character that is not a hash nor a space?".
If a user fat-fingers an unnecessary space into a blank line, that line
certainly will be excluded. But if the user fat-fingers ^X^I (or >> for vi
users), all lines begin with whitespace and they now get ignored?
How about removing the unnecessary negation from the logic and directly
ask what we really want to know?
That is, "Do we have a line that is _not_ comment?"
has_action () {
sane_grep -v -e '^#' -e '^[ ]*$' "$1" >/dev/null
}
Hmm?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html