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

Re: [PATCH RFC] cherry: support --abbrev option

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
Mar 16, 2010, 00:46 UTC
Message-ID
<4B9ED500.2040705@lsrfire.ath.cx>
In-Reply-To
<40aa078e1003151650m549bafdbwdd849e23349ce6a9@mail.gmail.com>
Am 16.03.2010 00:50, schrieb Erik Faye-Lund:
Show 31 quoted lines
> It's still just nitpicking, and I appreciate the feed-back. I'm a
> little bit hesitant here though, for the following reasons:
> - All other users of OPT__ABBREV (with the exception of ls-files,
> ls-tree and show-ref) initialize abbrev to it's default value (but
> they all use DEFAULT_ABBREV).
> - ls-files and ls-tree (but not show-ref) both does the following,
> when using abbrev: "abbrev ? find_unique_abbrev(ce->sha1,abbrev) :
> sha1_to_hex(ce->sha1)".
> - ls-files, ls-tree and show-ref, all seems to default "abbrev" to
> zero (by making it a static global).
> - I want to be consistent with the existing code.
> 
> So, basically, ls-files and ls-tree seems to think
> find_unique_abbrev() does not correctly (for this purpose) handle
> abbrev=0. However, show-ref does seem to assume so. Looking at the
> implementation of find_unique_abbrev(), it is clear that it does. But
> as I said: I want to be consistent, and the variation from show-ref
> (basically what you're suggesting) is the least common one.
> 
> So I guess I can either:
> 1) Change the code to be consistent with show-ref, and submit an
> additional patch to make ls-files and ls-tree consistent with this.
> This might have a performance-impact though, since
> find_unique_abbrev() does some extra work (checking the sha1 for
> existence and an extra buffer-copy).
> 2) Change the code as you suggest, and not care so much about consistency.
> 3) Leave the code to be functionally consistent with those who
> initialize abbrev to DEFAULT_ABBREV (but with a different default,
> which in itself is slightly inconsistent).
> 
> I'm leaning towards 3) for now, but I don't have any strong feelings.
If you do 2) then it stands 2:2 (ls-files, ls-tree vs. show-ref, cherry). :)

find_unique_abbrev() could be streamlined to degrade to sha1_to_hex() early on if len is 0 or >= 40, without any existence check or copy..

René
Previous: Erik Faye-LundNext: Erik Faye-Lund
Message 6 of 10 in “cherry: support --abbrev option”
  1. cherry: support --abbrev optionErik Faye-Lund, Mar 15, 2010
  2. René ScharfeMar 15, 2010
  3. Erik Faye-LundMar 15, 2010
  4. René ScharfeMar 15, 2010
  5. Erik Faye-LundMar 15, 2010
  6. René ScharfeMar 16, 2010
  7. Erik Faye-LundMar 16, 2010
  8. Junio C HamanoMar 16, 2010
  9. Junio C HamanoMar 15, 2010
  10. Erik Faye-LundMar 15, 2010

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.