git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Robert Fitzsimons <robfitz@273k.net>
Subject: Re: [PATCHv1bis 1/2] git apply: option to ignore whitespace  differences
Date: Thu, 2 Jul 2009 21:28:47 +0200	[thread overview]
Message-ID: <cb7bb73a0907021228q7e9d2791vafead8e0c5b06b79@mail.gmail.com> (raw)
In-Reply-To: <cb7bb73a0907021202ra322425pc64b54953f4f544d@mail.gmail.com>

On Thu, Jul 2, 2009 at 9:02 PM, Giuseppe
Bilotta<giuseppe.bilotta@gmail.com> wrote:
> On Thu, Jul 2, 2009 at 8:27 PM, Junio C Hamano<gitster@pobox.com> wrote:
>>> +/*
>>> + * Returns true if the given lines (buffer + len) match
>>> + * according to the ignore_whitespace setting
>>> + */
>>> +static int lines_match(const char *s1, size_t n1, const char *s2, size_t n2)
>>> +{
>>> +     if (ignore_whitespace)
>>> +             return !memcmp_ignore_whitespace(s1, n1, s2, n2);
>>> +     else
>>> +             return (n1 == n2) && !memcmp(s1, s2, n1);
>>> +}
>>> +
>>
>> I think this still is an abstraction at the wrong level.  For one thing,
>> if ignore-whitespace is set, you do not even need nor want to do the "fix
>> only the ws breakages we are going to fix anyway according to the ws_rule"
>> transformation applied to the preimage.
>
> I've thought some more about this, and you are right. We still want to
> ws fix the postimage, but that's done elsewhere.

Sorry for repying to myself here, but I'm not convinced again. Or to
be more specific: I think this kind of refactoring is totally out of
the scope of this patch. So although I agree with you in priciple, if
you don't mind I'll keep the first two patches simpler and less
invasive. I'll look into the refactoring as a third step.

-- 
Giuseppe "Oblomov" Bilotta

  reply	other threads:[~2009-07-02 19:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-02 17:48 [PATCHv1bis 0/2] git apply: cope with whitespace differences Giuseppe Bilotta
2009-07-02 17:48 ` [PATCHv1bis 1/2] git apply: option to ignore " Giuseppe Bilotta
2009-07-02 17:48   ` [PATCHv1bis 2/2] git apply: preserve original whitespace with --ignore-whitespace Giuseppe Bilotta
2009-07-02 18:27   ` [PATCHv1bis 1/2] git apply: option to ignore whitespace differences Junio C Hamano
2009-07-02 19:02     ` Giuseppe Bilotta
2009-07-02 19:28       ` Giuseppe Bilotta [this message]
2009-07-02 19:45         ` Junio C Hamano
2009-07-02 20:33           ` Giuseppe Bilotta
2009-07-02 21:00             ` Junio C Hamano
2009-07-02 21:05               ` Giuseppe Bilotta
2009-07-02 23:55             ` Junio C Hamano
2009-07-03  6:40               ` Giuseppe Bilotta

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=cb7bb73a0907021228q7e9d2791vafead8e0c5b06b79@mail.gmail.com \
    --to=giuseppe.bilotta@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=robfitz@273k.net \
    /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 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 NNTP newsgroup(s).