From: Junio C Hamano <gitster@pobox•com>
To: "Torsten Bögershausen" <tboegi@web•de>
Cc: Alexander Rinass <alex@fournova•com>,
Johannes Sixt <j6t@kdbg•org>,
git@vger•kernel.org
Subject: Re: [PATCH] diff: run arguments through precompose_argv
Date: Wed, 06 Apr 2016 10:27:31 -0700 [thread overview]
Message-ID: <xmqq60vuy7ek.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <5704BF14.9060100@web.de> ("Torsten Bögershausen"'s message of "Wed, 6 Apr 2016 09:47:32 +0200")
Torsten Bögershausen <tboegi@web•de> writes:
> May be something like this, (but this is highly a personal taste question)
My observation was not a suggestion to rewrite the log message, but
primarily about describing the change in behaviour.
I didn't mean to nitpick the phrasing but let me think aloud about
it, using this as a template. I rewrapped overlong lines from the
original.
> When running diff commands, file paths containing decomposed
> unicode code points are not converted to precomposed unicode form
> under Mac OS X.
>
> As a result, no diff is displayed.
Add ", but we normalize the paths in the index and the history to
precomposed form on that platform." at the end of the first sentence, to
make sure you mention both causes of "As a result" that follows.
Also, these are better to be a single paragraph. I.e.
When running diff commands, a pathspec containing decomposed
unicode code points is not converted to precomposed unicode form
under Mac OS X, but we normalize the paths in the index and the
history to precomposed form on that platform. As a result, the
pathspec would not match and no diff is shown.
> Opposite to most builtin commands, the diff builtin is missing the
> parse_options call, which internally runs arguments through the
> precompose_argv call, which ensures all arguments are in precomposed
> unicode form.
>
> Fix the problem by adding a precompose_argv call to diff,
> diff-index, diff-files and diff-tree.
Perhaps it is just me but I found "Opposite to most" harder to grok
for some reason.
And the "backward compatibility note" is missing. I'd write these
two paragraphs like this if I were doing this patch.
Unlike many builtin commands, the "diff" family of commands do
not use parse_options(), which is how other builtin commands
indirectly call precompose_argv() to normalize argv[] into
precomposed form on Mac OSX. Teach these commands to call
precompose_argv() themselves.
Note that precomopose_argv() normalizes not just paths but all
command line arguments, so things like "git diff -G $string"
when $string has the decomposed form would first normalized into
the precomposed form and would stop hitting the same string in
the decomposed form in the diff output with this change. It is
not a problem per-se, as "log" family of commands already use
parse_options() and call precompose_argv()--we can think of it
as making the "diff" family of commands behave in a similar way
as the commands in the "log" family.
next prev parent reply other threads:[~2016-04-06 17:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-04 20:38 [PATCH] diff: run arguments through precompose_argv Alexander Rinass
2016-04-04 20:38 ` Alexander Rinass
2016-04-05 17:09 ` Junio C Hamano
2016-04-05 19:15 ` Johannes Sixt
2016-04-05 19:27 ` Torsten Bögershausen
2016-04-05 21:42 ` Junio C Hamano
2016-04-06 6:51 ` Alexander Rinass
2016-04-06 7:47 ` Torsten Bögershausen
2016-04-06 17:27 ` Junio C Hamano [this message]
2016-05-11 22:08 ` Junio C Hamano
2016-05-12 11:16 ` Alexander Rinass
2016-05-12 15:39 ` Junio C Hamano
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=xmqq60vuy7ek.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=alex@fournova$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=j6t@kdbg$(echo .)org \
--cc=tboegi@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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