git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFH/PATCH] blame: tighten command line parser

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Feb 6, 2018, 04:42 UTC
Message-ID
<CAPig+cTFSWJX3FyXA6JJPhd1TT9eS0QOtm_R2qB6SX8W60ZWtA@mail.gmail.com>
In-Reply-To
<xmqqmv0n80ue.fsf@gitster-ct.c.googlers.com>
On Mon, Feb 5, 2018 at 6:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> The command line parser of "git blame" is prepared to take an
> ancient odd argument order "blame <path> <rev>" in addition to the
> usual "blame [<rev>] <path>".  It has at least two negative
> ramifications:
>
>  - In order to tell these two apart, it checks if the last command
>    line argument names a path in the working tree, using
>    file_exists().  However, "blame <rev> <path>" is a request to
>    explalin each and every line in the contents of <path> stored in
s/explalin/explain/
Show 8 quoted lines
>    revision <rev> and does not need to have a working tree version
>    of the file.  A check with file_exists() is simply wrong.
>
>  - To coerce that mistaken file_exists() check to work, the code
>    calls setup_work_tree() before doing so, because the path it has
>    is relative to the top-level of the project tree.  However,
>    "blame <rev> <path>" MUST be usable even in a bare repository,
>    and there is no reason for letting setup_work_tree() to complain
s/to complain/complain/
Show 19 quoted lines
>    and die with "This operation must be run in a work tree".
>
> To correct the former, switch to check if the last token is a
> revision (and if so, parse arguments using "blame <path> <rev>"
> rule).  Correct the latter by getting rid of setup_work_tree() and
> file_exists() check--the only case the call to this function matters
> is when we are running "blame <path>" (i.e. no starting revision and
> asking to blame the working tree file at <path>, digging through the
> HEAD revision), but there is a call in setup_scoreboard() just
> before it calls fake_working_tree_commit().
>
> Also attempt to give a bit more sensible error message when "blame
> XYZ" is given and XYZ cannot be a path.
>
>    side note: I am not happy with the "only one arg, which is a rev,
>    given in a bare repository" condition to give the new error
>    message, but this should be a good starting point.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Previous: Junio C Hamano
Message 2 of 2 in “blame: tighten command line parser”
  1. blame: tighten command line parserJunio C Hamano, Feb 5, 2018
  2. Eric SunshineFeb 6, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.