Re: [PATCH 3/2] attribute "coding": specify blob encoding

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 3/2] attribute "coding": specify blob encoding

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:02

しらいしななこ  [off-list ref] writes:
Quoting Junio C Hamano [off-list ref]:
quoted
This teaches "diff hunk header" function about custom character
encoding per path via gitattributes(5) so that it can sensibly
chomp a line without truncating a character in the middle.

Signed-off-by: Junio C Hamano <redacted>
---

 * This is not intended for serious inclusion, but was done more
   as a demonstration of the concept, hence [3/2].
Why not?  It looks a useful addition for us Japanese people.
    (offtopic) I was once told that "us Japanese people" is a
    bad thing to say in public because it sets an unfriendly
    tone by creating a psychological divide between "us" and
    "others".  After all I am one of you ;-)

The reason I do not like the patch as-is is because I have
doubts about the way "coding" acts in the patch.

There already is clean/smudge filter mechanism.  Even if your
work tree has files in euc-jp or Shift_JIS, you could choose to
internally use UTF-8 at git object level.  Then the part that
deals with diff hunk headers (the topic of the patch we are
discussing) would have to work only on UTF-8 data.

	Side note: when getting the data from a file in the work
	tree, we convert into internal representation before
	running diff (see diff.c::diff_populate_filespec()), but
	we do not convert it back to external representation by
	running the smudge filter on the diff output.  We might
	optionally want to but if somebody is going to do this,
	the patch accepting side also needs to be modified to
	reverse the conversion.

The solution with clean/smudge is not applicable to everybody.
It needs to be agreed upon project-wide what encoding is used as
the canonical encoding for the project, and when the project
chooses to use UTF-8, the above would become a cleaner and
workable approach.

If the project, on the other hand, chooses to use a non UTF-8
encoding (e.g. euc-jp) as the canonical representation,
something like my patch may be necessary.

Between these two ways to skin the cat, I do not want to close
the door for either one of them too early, although I am
somewhat partial to "internally everything is UTF-8" approach.

Maybe we would want to use "coding" (short, sweet and nice name
for an attribute) to mean a canned smudge/clean filter pair that
runs to/from UTF-8 iconv, making the "internally, everything is
UTF-8" approach a more officially supported option.  If we
choose to go that route, the way "coding" attribute was used in
the patch directly conflicts with that design, as the world view
my "coding" patch takes is "whatever coding project chooses is
used internally, and the attribute is used to teach coding
specific actions to the underlying logic".
quoted
+static struct {
+	const char *coding;
+	sane_truncate_fn fn;
+} builtin_truncate_fn[] = {
+	{ "euc-jp", truncate_euc_jp },
+	{ "utf-8", NULL },
+};
Can you also do JIS and Shift JIS?  I ask because many of my
old notes are in Shift JIS and I think it is the same for many
other people. 
I guess somebody else could (hint, hint,...).  Shift_JIS should
be more or less straightforward to add.

With the current code structure, however, ISO-2022 (you said
"JIS" -- Japanese often use that word to mean 7-bit ISO-2022 and
so did you in this context) is a bit cumbersome to handle, as
you cannot just truncate but also have to add a few escape bytes
to go back to ASCII at the end of line.

Re: [PATCH 3/2] attribute "coding": specify blob encoding

From: Tsugikazu Shibata <hidden>
Date: 2016-06-15 22:44:02

On Thu, 03 Jan 2008 13:54:58 -0800, gitster wrote:
しらいしななこ  [off-list ref] writes:
quoted
Quoting Junio C Hamano [off-list ref]:
quoted
This teaches "diff hunk header" function about custom character
encoding per path via gitattributes(5) so that it can sensibly
chomp a line without truncating a character in the middle.

Signed-off-by: Junio C Hamano <redacted>
---

 * This is not intended for serious inclusion, but was done more
   as a demonstration of the concept, hence [3/2].
...
quoted
quoted
+static struct {
+	const char *coding;
+	sane_truncate_fn fn;
+} builtin_truncate_fn[] = {
+	{ "euc-jp", truncate_euc_jp },
+	{ "utf-8", NULL },
+};
Can you also do JIS and Shift JIS?  I ask because many of my
old notes are in Shift JIS and I think it is the same for many
other people. 
I guess somebody else could (hint, hint,...).  Shift_JIS should
be more or less straightforward to add.

With the current code structure, however, ISO-2022 (you said
"JIS" -- Japanese often use that word to mean 7-bit ISO-2022 and
so did you in this context) is a bit cumbersome to handle, as
you cannot just truncate but also have to add a few escape bytes
to go back to ASCII at the end of line.
I guess that there are many other encodings and support everything are
not reasonable.
In my experience, It seems that chopping a multi-byte character
occurs the field after "@@". I believe this field is for the name of
the appropriate function. Also, I believe most of the function names
are in ASCII. 
So, question is why we should think of this field in case of non
programming language files ? 
In case of text file using any coding, Should we need to add something
after "@@"?
How about not to add anything after "@@" when the file name was .txt
or no extension (ie. HOWTO)?

Re: [PATCH 3/2] attribute "coding": specify blob encoding

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:02

Tsugikazu Shibata [off-list ref] writes:
So, question is why we should think of this field in case of non
programming language files ? 
In case of text file using any coding, Should we need to add something
after "@@"?
See gitattributes(5), "Defining a custom hunk-header".  The
associated configuration variable for this feature is somewhat
misnamed [*1*], but if you do not want to see anything, I think
you should be able to define a custom pattern that match
emptyness.

For .txt files, I always hoped that someday, somebody clever
would come up with a custom pattern to grok AsciiDoc mark-up to
pick a label-looking line so that the hunks in patches to our
documentation would be easier to find.  A simpler and more
generally applicable possibility is to find the previous line
that starts a paragraph (e.g. you would pick "For .txt files..."
for this part because it is the non-blank line that immediately
follows the first blank line before this part).  But I think the
code to find an appropriate string for hunk header is not
capable of looking at more than one line, so neither of these
would be easy to implement without changing the underlying logic
and semantics of "diff.*.funcname" configuration.

[Footnote]

*1* It may be misnamed but it is in line with GNU diff, though.
They call -p "show-c-function" and -F "show-function-line" --
both assume function names are what people are interested in
having on hunk headers.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help