git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Libor Pechacek <lpechacek@suse.cz>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Documentation fixes in git-config
Date: Thu, 20 Jan 2011 19:27:16 -0500	[thread overview]
Message-ID: <20110121002716.GC9442@sigill.intra.peff.net> (raw)
In-Reply-To: <20110119141401.GE8034@fm.suse.cz>

On Wed, Jan 19, 2011 at 03:14:01PM +0100, Libor Pechacek wrote:

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index ff7c225..0f23bc7 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -12,8 +12,9 @@ The configuration variables are used by both the git plumbing
>  and the porcelains. The variables are divided into sections, wherein
>  the fully qualified variable name of the variable itself is the last
>  dot-separated segment and the section name is everything before the last
> -dot. The variable names are case-insensitive and only alphanumeric
> -characters are allowed. Some variables may appear multiple times.
> +dot. The variable names are case-insensitive, only alphanumeric
> +characters and '-' are allowed and must start with an alphabetic character.
> +Some variables may appear multiple times.

The intent of the change looks fine, but your sentence doesn't quite
parse to me (to be fair, the problem is in the one you are replacing,
but adding the third clause makes it even more confusing). How about:

  The variables names are case-insensitive, allow only alphanumeric
  characters and '-', and must start with an alphabetic character.

>  --get-regexp::
>  	Like --get-all, but interprets the name as a regular expression.
> -	Also outputs the key names.
> +	Regular expression matching is case sensitive in all parts of the key,
> +	therefore make sure your pattern matches lower case letters in section
> +	and variable names.  Also outputs the key names.

That is only true because of the breakage in your first patch. Without
your patch, both of these work:

  git config --get-regexp 'Foo.*'
  git config --get-regexp 'foo.*'

That being said, the downcasing is extremely naive for regexps, and you
should try to match the canonical name. The current downcasing behavior
should probably stay for historical reasons, but is not well thought-out
(it may even be accidental). Perhaps we should therefore explain it in
those terms:

  Regular expression matching is case-sensitive and done against a
  canonicalized version of the key in which section and variable names
  are lowercased, but subsection names are not. For historical reasons,
  some simple regular expressions are lower-cased before matching
  (everything before the first dot and after the last dot), which makes
  things like "Core.*' work.

I dunno. Maybe we should just declare "Core.*' to be broken, and anybody
who was relying on it is wrong.

-Peff

  reply	other threads:[~2011-01-21  0:27 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-08 14:46 git-config does not check validity of variable names Libor Pechacek
2011-01-11  5:59 ` Jeff King
2011-01-19 10:01   ` Libor Pechacek
2011-01-19 14:11     ` [PATCH] Sanity-ckeck config " Libor Pechacek
2011-01-20 23:22       ` Jeff King
2011-01-21  0:06         ` Jeff King
2011-01-19 14:14     ` [PATCH] Documentation fixes in git-config Libor Pechacek
2011-01-21  0:27       ` Jeff King [this message]
2011-01-21 10:20         ` Libor Pechacek
2011-01-21 10:25           ` [PATCH v2] " Libor Pechacek
2011-01-21 16:25             ` Jeff King
2011-01-23 19:46               ` Libor Pechacek
2012-03-01  8:19             ` [PATCH v3] " Libor Pechacek
2012-03-01  9:08               ` Jeff King
2012-03-01 10:54                 ` Libor Pechacek
2012-03-01 16:24                 ` Junio C Hamano
2012-03-01 10:59               ` [PATCH v4] " Libor Pechacek
2011-01-21 10:02 ` [PATCH] Sanity-ckeck config variable names Libor Pechacek
2011-01-21 10:23   ` [PATCH v2] " Libor Pechacek
2011-01-21 16:23     ` Jeff King
2011-01-27 14:28       ` [PATCH v3] Sanity-check " Libor Pechacek
2011-01-27 22:45         ` Junio C Hamano
2011-01-28 14:53           ` Libor Pechacek
2011-01-30 19:40             ` [PATCH v4] " Libor Pechacek
2011-02-10 22:49               ` Junio C Hamano
2011-02-11 18:52                 ` Libor Pechacek
2011-01-27 14:52 ` [PATCH] Disallow empty section and " Libor Pechacek
2011-01-30 20:34   ` [PATCH v2] " Libor Pechacek
2011-01-31  7:48     ` Johannes Sixt
2011-01-31  9:17       ` Libor Pechacek
2011-01-31  9:29         ` Johannes Sixt
2011-01-31 13:08           ` [PATCH v3] " Libor Pechacek
2011-01-31 16:48             ` Jens Lehmann
2011-02-01  7:13               ` [PATCH v4] " Libor Pechacek
2011-02-10 22:49                 ` 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=20110121002716.GC9442@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=lpechacek@suse.cz \
    /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).