Re: [PATCH] log -L: improve error message on malformed argument
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 17, 2015, 20:38 UTC
- Message-ID
- <xmqqh9sepk20.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1429195387-20573-1-git-send-email-Matthieu.Moy@imag.fr>
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
Show 25 quoted lines
> The old message did not mention the :regex:file form.
>
> To avoid overly long lines, split the message into two lines (in case
> item->string is long, it will be the only part truncated in a narrow
> terminal).
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
> line-log.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/line-log.c b/line-log.c
> index a490efe..e725248 100644
> --- a/line-log.c
> +++ b/line-log.c
> @@ -575,7 +575,8 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)
>
> name_part = skip_range_arg(item->string);
> if (!name_part || *name_part != ':' || !name_part[1])
> - die("-L argument '%s' not of the form start,end:file",
> + die("invalid -L argument '%s'.\n"
> + "It should be of the form start,end:file or :regex:file.",
> item->string);
> range_part = xstrndup(item->string, name_part - item->string);
> name_part++;I actually think "start,end:file" is more correct than your updated text.
By adding :regex:file as a possibility, you are hinting that 'start' and 'end' are *not* regular expressions but numbers, but
$ git log -L'/^int main/,/^}/:git.c'
is a perfectly fine way to specify start (i.e. the first line that matches '^int main') and end (i.e. the first line that matches '^}' after that). Perhaps rephrase it as :funcname:file to avoid giving false impression to the other one, and use Eric's suggestion on top?
die("-L argument not 'start,end:file' or ':funcname:file': %s",
item->string);With the matching update to tests, here is what I'll queue on top of this patch for now, but please send in objections and improvements.
Thanks.
line-log.c | 3 +-- t/t4211-line-log.sh | 8 ++++---- 2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/line-log.c b/line-log.c index 5d4cb7c..51d9f7c 100644 --- a/line-log.c +++ b/line-log.c @@ -543,8 +543,7 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args) name_part = skip_range_arg(item->string); if (!name_part || *name_part != ':' || !name_part[1]) - die("invalid -L argument '%s'.\n" - "It should be of the form start,end:file or :regex:file.", + die("-L argument not 'start,end:file' or ':funcname:file': %s", item->string); range_part = xstrndup(item->string, name_part - item->string); name_part++; diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh index 426a828..edd5ed3 100755 --- a/t/t4211-line-log.sh +++ b/t/t4211-line-log.sh @@ -40,14 +40,14 @@ canned_test "-L 24,+1:a.c simple" vanishes-early canned_test "-L '/long f/,/^}/:b.c' move-support" move-support-f test_bad_opts "-L" "switch.*requires a value" -test_bad_opts "-L b.c" "argument.*not of the form" -test_bad_opts "-L 1:" "argument.*not of the form" +test_bad_opts "-L b.c" "argument not .start,end:file" +test_bad_opts "-L 1:" "argument not .start,end:file" test_bad_opts "-L 1:nonexistent" "There is no path" test_bad_opts "-L 1:simple" "There is no path" -test_bad_opts "-L '/foo:b.c'" "argument.*not of the form" +test_bad_opts "-L '/foo:b.c'" "argument not .start,end:file" test_bad_opts "-L 1000:b.c" "has only.*lines" test_bad_opts "-L 1,1000:b.c" "has only.*lines" -test_bad_opts "-L :b.c" "argument.*not of the form" +test_bad_opts "-L :b.c" "argument not .start,end:file" test_bad_opts "-L :foo:b.c" "no match" test_done
-- 2.4.0-rc2-183-g70401ab