llvm dev - Aug 2017 - [PATCH v2 0/2] clang-format

+ llvm-dev at lists.llvm.org

The Git community is currently discussing adopting a coding style
defined by clang-format, here is a bug report:

On Mon, Aug 14, 2017 at 4:06 PM, Jeff King <peff at peff.net>
wrote:
>
> One more oddity I found while playing with this that Git folks might run
> into:
>
>   $ git init tmp && cd tmp
>   $ git commit --allow-empty -m foo
>   $ echo "[mysection]mykey" >>.git/config
>   $ git clang-format-5.0
>   Traceback (most recent call last):
>     File "/usr/bin/git-clang-format-5.0", line 579, in
<module>
>       main()
>     File "/usr/bin/git-clang-format-5.0", line 62, in main
>       config = load_git_config()
>     File "/usr/bin/git-clang-format-5.0", line 194, in
load_git_config
>       name, value = entry.split('\n', 1)
>   ValueError: need more than 1 value to unpack
>
>   $ sed -i 's/mykey/&=true/' .git/config
>   $ git clang-format-5.0
>   no modified files to format
>
> So it looks like they do their own config parsing and it's not quite
> compatible. :(
>
> That's not the end of the world, and something we can try to fix
> upstream. I just wanted to mention it here so other people don't waste
> time trying to track down the problem.
>
> -Peff

On Mon, Aug 14, 2017 at 04:15:40PM -0700, Stefan Beller wrote:
> + llvm-dev at lists.llvm.org
> 
> The Git community is currently discussing adopting a coding style
> defined by clang-format, here is a bug report:
Since we've added a cc, let me try to give a little more context.
> > One more oddity I found while playing with this that Git folks might
run
> > into:
> >
> >   $ git init tmp && cd tmp
> >   $ git commit --allow-empty -m foo
> >   $ echo "[mysection]mykey" >>.git/config
> >   $ git clang-format-5.0
> >   Traceback (most recent call last):
> >     File "/usr/bin/git-clang-format-5.0", line 579, in
<module>
> >       main()
> >     File "/usr/bin/git-clang-format-5.0", line 62, in main
> >       config = load_git_config()
> >     File "/usr/bin/git-clang-format-5.0", line 194, in
load_git_config
> >       name, value = entry.split('\n', 1)
> >   ValueError: need more than 1 value to unpack
> >
> >   $ sed -i 's/mykey/&=true/' .git/config
> >   $ git clang-format-5.0
> >   no modified files to format
> >
> > So it looks like they do their own config parsing and it's not
quite
> > compatible. :(
In Git's config files, doing this:

  [mysection]
  mykey

is a shorthand for setting mysection.mkykey to "true". And the output
from "git config --list" will show just the keyname without a value,
like:

  mysection.mykey

instead of:

  some.key=this one has a value

There's a possible patch elsewhere in the thread:

  https://public-inbox.org/git/xmqqzib1sp6z.fsf at gitster.mtv.corp.google.com/

I'm happy to see it is running "git config --list", which means
it's
responding to syntactic funny-ness in the output of that command, not in
the original config (and other features like includes should Just Work
without the script caring).

I'm tempted to say that "config --list" should normalize this case
into:

  mysection.mykey=true

Normally we avoid coercing values without knowing the context in which
they'll be used. But the syntax in the original file means the user is
telling us it's a boolean and they expect it to be treated that way.

The only downside is if the user is wrong, it might be coerced into
the string "true" instead of throwing an error. That seems like a
minor
drawback for eliminating a potentially confusing corner case from the
plumbing output.

-Peff