From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: phillip.wood@dunelm.org.uk,
Matt R via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org, Matt R <mattr94@gmail.com>
Subject: Re: [PATCH 1/1] rebase -r: let `label` generate safer labels
Date: Tue, 03 Sep 2019 11:10:19 -0700 [thread overview]
Message-ID: <xmqqo901tfn8.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1909022124350.46@tvgsbejvaqbjf.bet> (Johannes Schindelin's message of "Mon, 2 Sep 2019 21:42:18 +0200 (CEST)")
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 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()?
next prev parent reply other threads:[~2019-09-03 18:10 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-02 14:01 [PATCH 0/1] Make git rebase -r's label generation more resilient Johannes Schindelin via GitGitGadget
2019-09-02 14:01 ` [PATCH 1/1] rebase -r: let `label` generate safer labels Matt R via GitGitGadget
2019-09-02 17:57 ` Phillip Wood
2019-09-02 18:29 ` Junio C Hamano
2019-09-02 20:12 ` brian m. carlson
2019-09-02 21:24 ` Philip Oakley
[not found] ` <CAOjrSZtw+wYHxFRQCfb80xzm9OsGDh2rW8uD+AYYdmDPxk5DFQ@mail.gmail.com>
2019-09-02 22:13 ` Philip Oakley
2019-09-03 11:19 ` Johannes Schindelin
2019-09-03 19:51 ` Junio C Hamano
2019-09-03 22:40 ` Matt Rogers
2019-09-02 19:42 ` Johannes Schindelin
2019-09-03 18:10 ` Junio C Hamano [this message]
2019-11-18 20:51 ` Johannes Schindelin
2019-11-17 23:16 ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Johannes Schindelin via GitGitGadget
2019-11-17 23:16 ` [PATCH v2 1/2] rebase-merges: move labels' whitespace mangling into `label_oid()` Johannes Schindelin via GitGitGadget
2019-11-17 23:16 ` [PATCH v2 2/2] rebase -r: let `label` generate safer labels Matthew Rogers via GitGitGadget
2019-11-18 3:42 ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Junio C Hamano
2019-11-18 11:23 ` [PATCH] sequencer: handle rebase-merge for "onto" message Danh Doan
2019-11-18 11:57 ` [PATCH v2] sequencer: handle rebase-merges " Doan Tran Cong Danh
2019-11-18 20:15 ` Johannes Schindelin
2019-11-21 0:16 ` [PATCH] sequencer: handle rebase-merge " Junio C Hamano
2019-11-18 20:52 ` [PATCH v2 0/2] Make git rebase -r's label generation more resilient Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqo901tfn8.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=mattr94@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).