| Message ID | 63f35287c9ced4d674f938bedd439aefa6c46f41.camel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | doc: fix quoting bug in credential cache example | expand |
douglas.fuller@gmail.com writes: > Unquoted semicolons are considered shell argument separators, quote > them so the example works correctly. I think what you wanted to do might make sense, but the above justification is totally incorrect. > # or you can specify your own shell snippet > -!f() { echo "password=`cat $HOME/.secret`"; }; f > +"!f() { echo password=`cat $HOME/.secret`; }; f" This is one of the examples shown, each shows possible value that can be given to credential.helper variable. Reproducing them fully: # run "git credential-foo" foo # same as above, but pass an argument to the helper foo --bar=baz # the arguments are parsed by the shell, so use shell # quoting if necessary foo --bar="whitespace arg" # you can also use an absolute path, which will not use the git wrapper /path/to/my/helper --with-arguments # or you can specify your own shell snippet !f() { echo "password=`cat $HOME/.secret`"; }; f These are examples of values, and how they may have to be quoted in various environments is not discussed here. We will not want a patch that says that the second example is wrong because "spaces separate arguments in shell and a string with a space in it must be quoted", i.e. $ git -c credential.helper="foo --bar=baz" frotz and does this # same as above, but pass an argument to the helper -foo --bar=baz +"foo --bar=baz" because the quoting convention would be different depending on where it appears. In a .git/config file, i.e. [credential] helper = foo --bar=baz is perfectly fine without quoting. $ git -c credential.helper='!f() { echo "password=`cat ...`"; }; f' frotz would be how you would pass a one-shot config from shell. Now, the reason why I said what you did is correct but the justification is wrong is because the semicolon does pose a problem in the .git/config file. In fact [credential] helper = !f() { echo "password=`cat ...`"; }; f would *NOT* work, because semicolon introduces a comment in the configuration file. For this particular case, you can just do [credential] helper = !echo password=`cat $HOME/.secret` without any quoting issues, though. Having said all that, I think we should clarify what these sample strings are in the introductory text in the examples. I've always thought that they are illustrating possible values and how to express that value in the context the values appear in is up to the readers who learn what values to write in this document (and they learn from manual for shell to learn the shell quoting convention and manual for 'git config' to learn the config quoting convention). Hence my initial reaction to your patch was "shell? Quoting for shell is outside the scope of the explanation here". On the other hand, for anybody who assumes that these examples are literally showing what you write after "[credential] helper = " in the configuration file, the example clearly is wrong and dq may be needed (but yours is also wrong, in that it loses double quotes around the argument to echo; if ~/.secret file had a tab in it, the helper will now yield a wrong password and you won't be able to log in).
On Thu, Apr 30, 2020 at 11:09:02AM -0700, Junio C Hamano wrote: > [...] > Having said all that, I think we should clarify what these sample > strings are in the introductory text in the examples. You already said everything I was going to. :) > I've always thought that they are illustrating possible values and > how to express that value in the context the values appear in is up > to the readers who learn what values to write in this document (and > they learn from manual for shell to learn the shell quoting > convention and manual for 'git config' to learn the config quoting > convention). Hence my initial reaction to your patch was "shell? > Quoting for shell is outside the scope of the explanation here". > > On the other hand, for anybody who assumes that these examples are > literally showing what you write after "[credential] helper = " in > the configuration file, the example clearly is wrong and dq may be > needed (but yours is also wrong, in that it loses double quotes > around the argument to echo; if ~/.secret file had a tab in it, the > helper will now yield a wrong password and you won't be able to log > in). Yes, they were definitely meant as: here are the raw values you would want to use, and it is up to you to figure out how to get that into a config file (whether on the cmdline via "git config" or editing the file yourself). I think we can either clarify that with a note at the beginning of the list, or we can just present it as config, like: diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt index 1814d2d23c..c756ecb8fd 100644 --- a/Documentation/gitcredentials.txt +++ b/Documentation/gitcredentials.txt @@ -216,20 +216,25 @@ Here are some example specifications: ---------------------------------------------------- # run "git credential-foo" -foo +[credential] +helper = foo # same as above, but pass an argument to the helper -foo --bar=baz +[credential] +helper = foo --bar=baz # the arguments are parsed by the shell, so use shell # quoting if necessary -foo --bar="whitespace arg" +[credential] +helper = "foo --bar='whitespace arg'" # you can also use an absolute path, which will not use the git wrapper -/path/to/my/helper --with-arguments +[credential] +helper = /path/to/my/helper --with-arguments # or you can specify your own shell snippet -!f() { echo "password=`cat $HOME/.secret`"; }; f +[credential] +helper = "!f() { echo \"password=`cat $HOME/.secret`\"; }; f" ---------------------------------------------------- Generally speaking, rule (3) above is the simplest for users to specify. It may be easier to just use double-quotes consistently, even for ones that do not need it, to give readers one less thing to wonder about. -Peff
On Fri, May 01, 2020 at 01:57:38AM -0400, Jeff King wrote: > It may be easier to just use double-quotes consistently, even for ones > that do not need it, to give readers one less thing to wonder about. So here's a patch that does that. I also noticed a few other deficiencies in that final example, which are fixed in the second patch. I hope I'm not stealing Douglas's thunder. :) [1/2]: gitcredentials(7): clarify quoting of helper examples [2/2]: gitcredentials(7): make shell-snippet example more realistic Documentation/gitcredentials.txt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) -Peff
Jeff King <peff@peff.net> writes: > I think we can either clarify that with a note at the beginning of the > list, or we can just present it as config, like: > > diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt > index 1814d2d23c..c756ecb8fd 100644 > --- a/Documentation/gitcredentials.txt > +++ b/Documentation/gitcredentials.txt > @@ -216,20 +216,25 @@ Here are some example specifications: > > ---------------------------------------------------- > # run "git credential-foo" > -foo > +[credential] > +helper = foo I like this style of "clarification"; it makes it clear that we are explaining the value in the config-file syntax. > # or you can specify your own shell snippet > -!f() { echo "password=`cat $HOME/.secret`"; }; f > +[credential] > +helper = "!f() { echo \"password=`cat $HOME/.secret`\"; }; f" But I do not think the "tentative shell function" trick is necessary. I personally think it is oversold ;-) For this particular one, [credential] helper = !echo \"password=`cat $HOME/.secret`\" should be sufficient, perhaps? You do not even need to understand how shell functions work to understand it. Also, "git config" indents the "var = value" lines, so it would look more natural if we indented our examples in a similar way. Thanks.
On Thu, Apr 30, 2020 at 11:20:52PM -0700, Junio C Hamano wrote: > > # or you can specify your own shell snippet > > -!f() { echo "password=`cat $HOME/.secret`"; }; f > > +[credential] > > +helper = "!f() { echo \"password=`cat $HOME/.secret`\"; }; f" > > But I do not think the "tentative shell function" trick is > necessary. I personally think it is oversold ;-) For this > particular one, > > [credential] > helper = !echo \"password=`cat $HOME/.secret`\" > > should be sufficient, perhaps? You do not even need to understand > how shell functions work to understand it. That will print: password=your-password get See the patches I just sent for a better example. :) > Also, "git config" indents the "var = value" lines, so it would look > more natural if we indented our examples in a similar way. We do earlier, too. I can re-indent the patches I just set, but I'll hold off on sending in case you have any other comment. -Peff
On Fri, 2020-05-01 at 02:19 -0400, Jeff King wrote: > On Fri, May 01, 2020 at 01:57:38AM -0400, Jeff King wrote: > > > It may be easier to just use double-quotes consistently, even for > > ones > > that do not need it, to give readers one less thing to wonder > > about. > > So here's a patch that does that. I also noticed a few other > deficiencies in that final example, which are fixed in the second > patch. > > I hope I'm not stealing Douglas's thunder. :) My one-line patch definitely wasn't thunderous -- I didn't even know how to write a comment in a config file. As posited upthread, I did interpret the example as presenting config file syntax, so I got tripped up by the semicolon there. This patch makes it more clear. I use git (thanks!) without having looked at the code, so I figured I was a good example of the target audience for this doc. Thanks for making it more clear. Cheers, --Doug > [1/2]: gitcredentials(7): clarify quoting of helper examples > [2/2]: gitcredentials(7): make shell-snippet example more realistic > > Documentation/gitcredentials.txt | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > -Peff
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt index 1814d2d23c..fb16c71cfe 100644 --- a/Documentation/gitcredentials.txt +++ b/Documentation/gitcredentials.txt @@ -229,7 +229,7 @@ foo --bar="whitespace arg" /path/to/my/helper --with-arguments # or you can specify your own shell snippet -!f() { echo "password=`cat $HOME/.secret`"; }; f +"!f() { echo password=`cat $HOME/.secret`; }; f" ---------------------------------------------------- Generally speaking, rule (3) above is the simplest for users to
Unquoted semicolons are considered shell argument separators, quote them so the example works correctly. Signed-off-by: Douglas Fuller <douglas.fuller@gmail.com> --- Documentation/gitcredentials.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) specify.