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

Re: [PATCH 1/1] rebase -r: let `label` generate safer labels

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 3, 2019, 18:10 UTC
Message-ID
<xmqqo901tfn8.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<nycvar.QRO.7.76.6.1909022124350.46@tvgsbejvaqbjf.bet>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 8 quoted lines
>> I'm sightly concerned that this opens the possibility for unexpected effects
>> if two different labels get sanitized to the same string. I suspect it's
>> unlikely to happen in practice but doing something like percent encoding
>> non-alphanumeric characters would avoid the problem entirely.
>
> Oh, but we make sure that the labels are unique, via the `label_oid()`
> function! Otherwise, we would not be able to label more than one merge
> parent ;-)

It somewhat feels suboptimal, from code followability's point of view, to have this "pre-sanitization" to replace isspace() to a dash, which is being extended to "all non-alnums", and the uniquefy of labels in label_oid(), in two separate places. I wonder if the resulting code becomes easier to follow and harder to introduce new bugs, if this part is made to just yield label.buf it obtained form the log message as-is and leave the munging to label_oid()?

Previous: Johannes SchindelinNext: Johannes Schindelin
Message 11 of 22 in “Make git rebase -r's label generation more resilient”
  1. 0/1 Make git rebase -r's label generation more resilientJohannes Schindelin via GitGitGadget, Sep 2, 2019
  2. 1/1 rebase -r: let `label` generate safer labelsMatt R via GitGitGadget, Sep 2, 2019
  3. Phillip WoodSep 2, 2019
  4. Junio C HamanoSep 2, 2019
  5. brian m. carlsonSep 2, 2019
  6. Philip OakleySep 2, 2019
  7. Johannes SchindelinSep 3, 2019
  8. Junio C HamanoSep 3, 2019
  9. Matt RogersSep 3, 2019
  10. Johannes SchindelinSep 2, 2019
  11. Junio C HamanoSep 3, 2019
  12. Johannes SchindelinNov 18, 2019
  13. 0/2 Make git rebase -r's label generation more resilientJohannes Schindelin via GitGitGadget, Nov 17, 2019
  14. 2/2 rebase -r: let `label` generate safer labelsMatthew Rogers via GitGitGadget, Nov 17, 2019
  15. 1/2 rebase-merges: move labels' whitespace mangling into `label_oid()`Johannes Schindelin via GitGitGadget, Nov 17, 2019
  16. Junio C HamanoNov 18, 2019
  17. sequencer: handle rebase-merge for "onto" messageDanh Doan, Nov 18, 2019
  18. sequencer: handle rebase-merges for "onto" messageDoan Tran Cong Danh, Nov 18, 2019
  19. Johannes SchindelinNov 18, 2019
  20. Junio C HamanoNov 21, 2019
  21. Johannes SchindelinNov 18, 2019
  22. Philip OakleySep 2, 2019

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.