git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Jeff King <peff@peff.net>, Stas Bekman <stas@stason.org>,
	git@vger.kernel.org
Subject: Re: git silently ignores include directive with single quotes
Date: Mon, 10 Sep 2018 12:17:58 -0700	[thread overview]
Message-ID: <xmqq1sa1tb4p.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180910183557.GD26356@aiede.svl.corp.google.com> (Jonathan Nieder's message of "Mon, 10 Sep 2018 11:35:57 -0700")

Jonathan Nieder <jrnieder@gmail.com> writes:

> Updated proposal:
>
>   1. Treat strings starting or ending with single-quote as worth
>      quoting in config.c::write_pair (line 3269).  This would already
>      help with the original issue, since the config would say
>
> 	[foo]
> 		bar = "'baz'"
>
>      allowing a quick diagnosis.

This does not change anything essential from Git's point of view
since the previous round.

But I changed my mind anyway ;-)  Earlier I said "if we do not
intend to make sq special, we shouldn't do #1", and it still is a
good direction to go.

But making sq special does not have to be making sq a character that
quotes.  A character (or a character sequence) that is *not* quoting
can still be special---for example, we can say certain character or
a character sequence *must* be quoted, and make sq such a character.

That is, even if we stop at your step 3. or step 2., and did not go
to step 4. (which I think is a bad idea for little gain), we are
already treating sq as a special character, and step 1. above is a
reasonable way to start the transition to that better world.

The reason why it is a better world that have "must be quoted, even
though they are not quoting characters themselves" is solely because
that would avoid confusion for those who are not familiar with the
file format, even when we stop at step #2.  In addition to a single
quote a the beginning of the value, I think two or more SP deserve
to be such a "must be quoted" sequence, i.e. instead of producing
this result, which we see with today's Git:

	$ git config a.test0 "'foo"
	$ git config a.test1 "foo  bar" ;# two spaces
	$ grep -A2 '\[a\]' config
	[a]
		test0 = 'foo
		test1 = foo  bar

we'd produce

	$ grep -A2 '\[a\]' config
	[a]
		test0 = "'foo"
		test1 = "foo  bar"

but we can still interpret what we have historically written the
same way.

I do not know if step #3 is a good idea, and I do not think step #2
is particularly a good stopping point.  Step #1 is probably slightly
a better stopping point if the aim is to avoid user confusion than
step #2.


>   2. (optional) Warn if a value is surrounded in single-quotes,
>      encouraging using surrounding double-quotes to disambiguate.
>
>   3. (optional) Error out if a value is surrounded in single-quotes,
>      encouraging replacing with or surrounding with double-quote,
>      depending on the user's intention.
>
>   4. (optional) Start treating wrapping single-quotes specially
>      somehow.
>
> As before, I think step 1 is a good idea, but I'm not convinced about
> any of the later steps.
>
> Thanks,
> Jonathan

  reply	other threads:[~2018-09-10 19:18 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-08 18:58 git silently ignores include directive with single quotes Stas Bekman
2018-09-08 19:30 ` Martin Ågren
2018-09-08 19:44   ` Stas Bekman
2018-09-08 19:53   ` Stas Bekman
2018-09-08 20:02     ` Ævar Arnfjörð Bjarmason
2018-09-08 20:13       ` Stas Bekman
2018-09-08 20:28         ` Ævar Arnfjörð Bjarmason
2018-09-08 20:58           ` Stas Bekman
2018-09-09  2:51         ` Paul Smith
2018-09-09  2:57           ` Stas Bekman
2018-09-08 19:54   ` Ævar Arnfjörð Bjarmason
2018-09-08 20:04     ` Stas Bekman
2018-09-08 20:16       ` Ævar Arnfjörð Bjarmason
2018-09-08 21:14     ` Jeff King
2018-09-08 22:10       ` Ramsay Jones
2018-09-09  2:33         ` Jeff King
2018-09-11 20:36           ` Junio C Hamano
2018-09-11 20:57             ` Jeff King
2018-09-23 22:48               ` Stas Bekman
2018-09-24 21:08                 ` Ævar Arnfjörð Bjarmason
2018-09-24 23:20                   ` Stas Bekman
2018-09-24 22:24                 ` [PATCH 0/1] " Philip Oakley
2018-09-24 23:05                   ` Stas Bekman
2018-09-24 22:24                 ` [PATCH 1/1] config doc: highlight the name=value syntax Philip Oakley
2018-09-25 22:03                   ` Junio C Hamano
2018-09-08 22:32       ` git silently ignores include directive with single quotes Ævar Arnfjörð Bjarmason
2018-09-09  2:29         ` Jeff King
2018-09-08 21:22 ` Jeff King
2018-09-08 22:49   ` Stas Bekman
2018-09-09  2:30     ` Jeff King
2018-09-10 17:04   ` Junio C Hamano
2018-09-10 17:14     ` Jonathan Nieder
2018-09-10 18:30       ` Junio C Hamano
2018-09-10 18:35         ` Jonathan Nieder
2018-09-10 19:17           ` Junio C Hamano [this message]
2018-09-10 19:52             ` Stas Bekman
2018-09-10 20:10               ` 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

  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=xmqq1sa1tb4p.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=stas@stason.org \
    /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).