Re: [PATCH v2 20/23] rebase -i: parse to-do list command line options
Hi Thomas,
an updated patch is attached below.
Thomas Rast writes: Fabian Ruch baf...@gmail.com writes:
[...]
are not supported at the moment. Neither are options that contain
spaces because the shell expansion of `args` in `do_next` interprets
white space characters as argument separator, that is a command line
like
pick --author A U Thor fa1afe1 Some change
is parsed as the pick command
pick --author
and the commit hash
A
which obviously results in an unknown revision error. For the sake of
completeness, in the example above the message title variable `rest`
is assigned the string 'U Thor fa1afe1 Some change' (without the
single quotes).
You could probably trim down the non-example a bit and instead give an
example :-)
Done. The example reword --signoff is substituted for the
non-example and used for an error message example as well. I hope
that is not confusing.
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment.
Can you add a test that verifies we catch an obvious unknown option
(such as --unknown-option)?
Done. The test checks that git-rebase--interactive fails to execute
'pick --unknown-option' and that the rebase can be resumed after
removing the --unknown-option part.
Cleanly break the `do_next` loop by assigning the special
value 'unknown' to the local variable `command`, which triggers the
unknown command case in `do_cmd`.
[...]
do_replay () {
command=$1
-sha1=$2
-rest=$3
+shift
+
+opts=
+while test $# -gt 0
+do
+case $1 in
+-*)
+warn Unknown option: $1
+command=unknown
+;;
+*)
+break
+;;
This seems a rather hacky solution to me. Doesn't this now print
warning: Unknown option: --unknown-option
warning: Unknown command: pick --unknown-option
?
It shouldn't claim the command is unknown if the command itself was
valid.
OK. do_replay now first checks for unknown commands and exits
immediately if that is the case, ignoring any options specified.
Also, you speak of do_cmd above, but the unknown command handling seems
to be part of do_replay?
Fixed.
Fabian
-- 8 --
Subject: rebase -i: parse to-do list command line options
Read in to-do list lines as
command args
instead of
command sha1 rest
so that to-do list command lines can specify additional arguments
apart from the commit hash and the log message title, which become
the non-options in `args`. Loop over `args`, put all options (an
argument beginning with a dash) in `opts`, stop the loop on the first
non-option and assign it to `sha1`. For instance, the to-do list
reword --signoff fa1afe1 Some change
is parsed as `command=reword`, `opts= '--signoff'` (including the
single quotes and the space for evaluation and concatenation of
options), `sha1=fa1afel` and `rest=Some change`. The loop does not
know the options it parses so that options that take an argument
themselves are not supported at the moment. Neither are options that
contain spaces because the shell expansion of `args` in `do_next`
interprets white space characters as argument separator.
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment. Cleanly break the `do_next` loop by assigning a reason to the
local variable `malformed`, which triggers the unknown command code
in `do_replay`. Move the error code to the beginning of `do_replay`
so that unknown commands are reported before bad options are as only
the first error we come across is reported. For instance, the to-do
list from above produces the error
Unknown 'reword' option: --signoff
Please fix this using 'git rebase --edit-todo'.
The to-do list is also parsed when the commit hashes are translated
between long and short format before and after the to-do list is
edited. Apply the same procedure as in `do_replay` with the exception
that we only care about where the options stop and the commit hash
begins. Do not reject any options when transforming the commit
hashes.
Enable the specification of to-do list command line options in
`FAKE_LINES` the same way this is accomplished for command lines
passed to `exec`. Define a new `fake_editor.sh` that edits a static
to-do list instead of the one passed as argument when invoked by
git-rebase. Add a test case that checks that unknown options are
refused and can be corrected using `--edit-todo`.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 80 --
t/lib-rebase.sh| 20 +--
t/t3427-rebase-line-options.sh | 26 ++
3 files changed, 105 insertions(+), 21 deletions(-)
create mode 100755 t/t3427-rebase-line-options.sh
diff --git
Re: [PATCH v2 20/23] rebase -i: parse to-do list command line options
Fabian Ruch baf...@gmail.com writes:
[...]
are not supported at the moment. Neither are options that contain
spaces because the shell expansion of `args` in `do_next` interprets
white space characters as argument separator, that is a command line
like
pick --author A U Thor fa1afe1 Some change
is parsed as the pick command
pick --author
and the commit hash
A
which obviously results in an unknown revision error. For the sake of
completeness, in the example above the message title variable `rest`
is assigned the string 'U Thor fa1afe1 Some change' (without the
single quotes).
You could probably trim down the non-example a bit and instead give an
example :-)
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment.
Can you add a test that verifies we catch an obvious unknown option
(such as --unknown-option)?
Cleanly break the `do_next` loop by assigning the special
value 'unknown' to the local variable `command`, which triggers the
unknown command case in `do_cmd`.
[...]
do_replay () {
command=$1
- sha1=$2
- rest=$3
+ shift
+
+ opts=
+ while test $# -gt 0
+ do
+ case $1 in
+ -*)
+ warn Unknown option: $1
+ command=unknown
+ ;;
+ *)
+ break
+ ;;
This seems a rather hacky solution to me. Doesn't this now print
warning: Unknown option: --unknown-option
warning: Unknown command: pick --unknown-option
?
It shouldn't claim the command is unknown if the command itself was
valid.
Also, you speak of do_cmd above, but the unknown command handling seems
to be part of do_replay?
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 20/23] rebase -i: parse to-do list command line options
Read in to-do list lines as
command args
instead of
command sha1 rest
so that to-do list command lines can specify additional arguments
apart from the commit hash and the log message title, which become
the non-options in `args`. Loop over `args`, put all options (an
argument beginning with a dash) in `opts`, stop the loop on the first
non-option and assign it to `sha1`. The loop does not know the
options it parses so that options that take an argument themselves
are not supported at the moment. Neither are options that contain
spaces because the shell expansion of `args` in `do_next` interprets
white space characters as argument separator, that is a command line
like
pick --author A U Thor fa1afe1 Some change
is parsed as the pick command
pick --author
and the commit hash
A
which obviously results in an unknown revision error. For the sake of
completeness, in the example above the message title variable `rest`
is assigned the string 'U Thor fa1afe1 Some change' (without the
single quotes).
Print an error message for unknown or unsupported command line
options, which means an error for all specified options at the
moment. Cleanly break the `do_next` loop by assigning the special
value 'unknown' to the local variable `command`, which triggers the
unknown command case in `do_cmd`.
The to-do list is also parsed when the commit hashes are translated
between long and short format before and after the to-do list is
edited. Apply the same procedure as in `do_cmd` with the exception
that we only care about where the options stop and the commit hash
begins. Do not reject any options when transforming the commit
hashes.
Signed-off-by: Fabian Ruch baf...@gmail.com
---
git-rebase--interactive.sh | 49 ++
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ecd4d7..da435cb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -577,8 +577,26 @@ do_pick () {
do_replay () {
command=$1
- sha1=$2
- rest=$3
+ shift
+
+ opts=
+ while test $# -gt 0
+ do
+ case $1 in
+ -*)
+ warn Unknown option: $1
+ command=unknown
+ ;;
+ *)
+ break
+ ;;
+ esac
+ opts=$opts $(git rev-parse --sq-quote $1)
+ shift
+ done
+ sha1=$1
+ shift
+ rest=$*
case $command in
pick|p)
@@ -675,7 +693,7 @@ do_replay () {
do_next () {
rm -f $msg $author_script $amend || exit
- read -r command sha1 rest $todo
+ read -r command args $todo
case $command in
$comment_char*|''|noop)
@@ -720,7 +738,7 @@ do_next () {
fi
;;
*)
- do_replay $command $sha1 $rest
+ do_replay $command $args
;;
esac
test -s $todo return
@@ -800,19 +818,34 @@ skip_unnecessary_picks () {
}
transform_todo_ids () {
- while read -r command rest
+ while read -r command args
do
case $command in
$comment_char* | exec)
# Be careful for oddball commands like 'exec'
# that do not have a SHA-1 at the beginning of $rest.
+ newargs=\ $args
;;
*)
- sha1=$(git rev-parse --verify --quiet $@ ${rest%% *})
- rest=$sha1 ${rest#* }
+ newargs=
+ sha1=
+ for arg in $args
+ do
+ case $arg in
+ -*)
+ newargs=$newargs $arg
+ ;;
+ *)
+ test -z $sha1
+ sha1=$(git rev-parse --verify
--quiet $@ $arg)
+ arg=$sha1
+ newargs=$newargs $arg
+ ;;
+ esac
+ done
;;
esac
- printf '%s\n' $command${rest:+ }$rest
+ printf '%s\n' $command$newargs
done $todo $todo.new
mv -f $todo.new $todo
}
--
2.0.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
