All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>,
	Git List <git@vger.kernel.org>,
	Remi Lespinet <remi.lespinet@ensimag.grenoble-inp.fr>,
	Guillaume Pages <guillaume.pages@ensimag.grenoble-inp.fr>,
	Louis-Alexandre Stuber 
	<louis--alexandre.stuber@ensimag.grenoble-inp.fr>,
	Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr>
Subject: Re: [PATCH v5 3/4] status: give more information during rebase -i
Date: Wed, 01 Jul 2015 09:36:24 -0700	[thread overview]
Message-ID: <xmqqvbe3al87.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cSXzN84C4tOi5oZaK_1UJGcNXywE_-V7CyDQUMXox1Acw@mail.gmail.com> (Eric Sunshine's message of "Wed, 1 Jul 2015 12:33:03 -0400")

Eric Sunshine <sunshine@sunshineco.com> writes:

> I was about to mention the same shortcoming, but you beat me to it.
>
>> Perhaps be more strict and do this instead (without
>> leading strbuf_trim):
>>
>>         if (!get_sha1_hex(split[1]->buf, sha1) &&
>>             !strcmp(split[1]->buf + 40, " ") {
>>                 replace split[1] with "%s " abbrev
>>         }
>
> Isn't it dangerous to assume that you can index 40 characters into
> split[1]? If (for some reason), the user botched the todo line such
> that the SHA1 is no longer a valid hex string, then split[1] will be
> that botched string, which might be shorter than 40 characters. For
> instance, if the user-edited todo line is:
>
>     pick oops nothing
>
> then git-rebase--interactive.sh:transform_todo_ids() will leave "oops"
> as-is, since it can't unabbreviate it, and then this code will place
> "oops" into split[1].

Yeah, that is why get_sha1_hex() is checked before we try to make
sure buf[40] has " " in the code snippet you quoted.

Isn't that how && short-cut works?

  reply	other threads:[~2015-07-01 16:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-30 13:01 [PATCH v4 0/4] More helpful 'git status' during 'rebase -i' Matthieu Moy
2015-06-30 13:01 ` [PATCH v4 1/4] status: factor two rebase-related messages together Matthieu Moy
2015-06-30 13:01 ` [PATCH v4 2/4] status: differentiate interactive from non-interactive rebases Matthieu Moy
2015-06-30 13:01 ` [PATCH v4 3/4] status: give more information during rebase -i Matthieu Moy
2015-06-30 13:01 ` [PATCH v4 4/4] status: add new tests for status " Matthieu Moy
2015-06-30 23:03 ` [PATCH v4 0/4] More helpful 'git status' during 'rebase -i' Junio C Hamano
2015-07-01  8:12   ` Matthieu Moy
2015-07-01  8:30     ` [PATCH v5 1/4] status: factor two rebase-related messages together Matthieu Moy
2015-07-01  8:30       ` [PATCH v5 2/4] status: differentiate interactive from non-interactive rebases Matthieu Moy
2015-07-01  8:30       ` [PATCH v5 3/4] status: give more information during rebase -i Matthieu Moy
2015-07-01 16:18         ` Junio C Hamano
2015-07-01 16:33           ` Eric Sunshine
2015-07-01 16:36             ` Junio C Hamano [this message]
2015-07-01 16:37               ` Eric Sunshine
2015-07-01 21:06           ` Matthieu Moy
2015-07-01 21:08             ` [PATCH v6 1/4] status: factor two rebase-related messages together Matthieu Moy
2015-07-01 21:08               ` [PATCH v6 2/4] status: differentiate interactive from non-interactive rebases Matthieu Moy
2015-07-01 21:08               ` [PATCH v6 3/4] status: give more information during rebase -i Matthieu Moy
2015-07-07 22:14                 ` Junio C Hamano
2015-07-08  7:10                   ` Matthieu Moy
2015-07-01 21:08               ` [PATCH v6 4/4] status: add new tests for status " Matthieu Moy
2015-07-01 21:23             ` [PATCH v5 3/4] status: give more information " Junio C Hamano
2015-07-02  8:01               ` Matthieu Moy
2015-07-03 17:40                 ` Junio C Hamano
2015-07-02  8:10               ` Matthieu Moy
2015-07-01  8:30       ` [PATCH v5 4/4] status: add new tests for status " Matthieu Moy

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvbe3al87.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Matthieu.Moy@imag.fr \
    --cc=antoine.delaite@ensimag.grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=guillaume.pages@ensimag.grenoble-inp.fr \
    --cc=louis--alexandre.stuber@ensimag.grenoble-inp.fr \
    --cc=remi.lespinet@ensimag.grenoble-inp.fr \
    --cc=sunshine@sunshineco.com \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.