git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v7 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type`

From
Taylor Blau <me@ttaylorr.com>
Date
Apr 10, 2018, 02:17 UTC
Message-ID
<20180410021757.GB937@syl.local>
In-Reply-To
<xmqqsh834yz1.fsf@gitster-ct.c.googlers.com>
On Tue, Apr 10, 2018 at 10:44:18AM +0900, Junio C Hamano wrote:
Show 10 quoted lines
> Taylor Blau <me@ttaylorr.com> writes:
>
> > `git config` has long allowed the ability for callers to provide a 'type
> > specifier', which instructs `git config` to (1) ensure that incoming
> > values are satisfiable under that type, and (2) that outgoing values are
> > canonicalized under that type.
>
> Hmm, "satisfiable" is not the first word that comes to my mind to
> describe "when you give me a string 'frotz', we cannot take it as an
> integer".  s/are satisfiable under/can be interpreted as/ perhaps?

Sure; I think that "can be interpreted as" is clearer. I have amended my patch locally, and will include this in the next re-roll, should one exist.

Show 17 quoted lines
> > In another series, we propose to extend this functionality with
> > `--type=color` and `--default` to replace `--get-color`.
> >
> > However, we traditionally use `--color` to mean "colorize this output",
> > instead of "this value should be treated as a color".
>
> Makes sense.
>
> > Currently, `git config` does not support this kind of colorization, but
> > we should be careful to avoid inhabiting this option too soon, so that
> > `git config` can support `--type=color` (in the traditional sense) in
> > the future, if that is desired.
>
> Shouldn't the above `--color` instead?  That is, we avoid squatting
> on `--color` because we might later want to use it in the
> traditional way.  Also my reading stuttered around "inhabiting".  It
> might be just me, but perhaps s/inhabiting/squating on/?
Ack, yes to both.
Show 37 quoted lines
> > In this patch, we support `--type=<int|bool|bool-or-int|...>` in
> > addition to `--int`, `--bool`, and etc. This allows the aforementioned
> > upcoming patch to support querying a color value with a default via
> > `--type=color --default=...`, without squandering `--color`.
>
> Good.
>
> > @@ -160,30 +158,39 @@ See also <<FILES>>.
> >  --list::
> >  	List all variables set in config file, along with their values.
> >
> > ---bool::
> > -	'git config' will ensure that the output is "true" or "false"
> > +--type <type>::
> > +  'git config' will ensure that any input or output is valid under the given
> > +  type constraint(s), and will canonicalize outgoing values in `<type>`'s
> > +  canonical form.
> > ++
> > +Valid `<type>`'s include:
> > ++
> > +- 'bool': canonicalize values as either "true" or "false".
> > +- 'int': canonicalize values as simple decimal numbers. An optional suffix of
> > +  'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or
> > +  1073741824 prior to output.
>
> It is not a new problem introduced by this patch, but these place
> too much weight on the output side and may end up giving a flawed
> mental model to the readers, I am afraid.
>
> It's not like an 'int' value internally is "2k" and shown it as 2048
> when output.  In reality, we internalize it 2048 upon input and we
> do not do anything special when showing, no?  A 'bool' value given
> with a string 'no' internally becomes 0 upon input and is shown as
> 'false' upon output.
>
> I suspect s/prior to output/upon input/ may alleviate the issue
> quite a bit.

I share your concern, and I think that your suggestion would alleviate the issue entirely.

I have applied these suggestions locally. :-)

Thanks, Taylor

Previous: Junio C HamanoNext: Taylor Blau
Message 54 of 60 in “builtin/config.c: prefer `--type=bool` over `--bool`, etc.”
  1. builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Mar 28, 2018
  2. Junio C HamanoMar 29, 2018
  3. Jeff KingMar 29, 2018
  4. Taylor BlauMar 30, 2018
  5. Jeff KingMar 30, 2018
  6. Junio C HamanoMar 30, 2018
  7. Jeff KingMar 30, 2018
  8. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Mar 30, 2018
  9. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Mar 30, 2018
  10. René ScharfeMar 30, 2018
  11. Jeff KingMar 30, 2018
  12. Jeff KingMar 30, 2018
  13. 0/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 4, 2018
  14. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 4, 2018
  15. Eric SunshineApr 4, 2018
  16. Taylor BlauApr 5, 2018
  17. Jeff KingApr 5, 2018
  18. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 4, 2018
  19. Eric SunshineApr 4, 2018
  20. Taylor BlauApr 5, 2018
  21. 0/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  22. Jeff KingApr 5, 2018
  23. Taylor BlauApr 5, 2018
  24. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 6, 2018
  25. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 9, 2018
  26. Eric SunshineApr 9, 2018
  27. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 11, 2018
  28. Junio C HamanoApr 11, 2018
  29. Taylor BlauApr 11, 2018
  30. Junio C HamanoApr 11, 2018
  31. Taylor BlauApr 11, 2018
  32. 0/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 18, 2018
  33. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 5, 2018
  34. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  35. Jeff KingApr 5, 2018
  36. Taylor BlauApr 5, 2018
  37. Taylor BlauApr 6, 2018
  38. 2/2 builtin/config.c: prefer `--type=bool` over `--bool`, etc.Taylor Blau, Apr 5, 2018
  39. Eric SunshineApr 5, 2018
  40. Jeff KingApr 5, 2018
  41. Taylor BlauApr 6, 2018
  42. Taylor BlauApr 6, 2018
  43. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 6, 2018
  44. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 6, 2018
  45. Eric SunshineApr 6, 2018
  46. Taylor BlauApr 7, 2018
  47. Eric SunshineApr 7, 2018
  48. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 9, 2018
  49. Junio C HamanoApr 10, 2018
  50. Taylor BlauApr 10, 2018
  51. Eric SunshineApr 10, 2018
  52. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 9, 2018
  53. Junio C HamanoApr 10, 2018
  54. Taylor BlauApr 10, 2018
  55. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 11, 2018
  56. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 11, 2018
  57. 1/2 builtin/config.c: treat type specifiers singularlyTaylor Blau, Apr 18, 2018
  58. 2/2 builtin/config.c: support `--type=<type>` as preferred alias for `--type`Taylor Blau, Apr 18, 2018
  59. Junio C HamanoApr 19, 2018
  60. Taylor BlauApr 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.