Re: [PATCH] Allow git-config to append a comment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Junio C Hamano:

> Make sure your title will mix well in "git shortlog --no-merges"
> output from recent commits by other contributors.

Thank you for your in-depth comment. This is the first time I have
ever considered contributing to Git, so I have a lot to learn. My pull
request [1] on GitGitGadget has been approved by Johannes Schindelin,
by the way, and the PR is based on an issue Johannes created [2] after
a brief discussion him and I had on Discord [3]. I have updated the
subject line, as you suggested.

  [1] https://github.com/gitgitgadget/git/pull/1681
  [2] https://github.com/gitgitgadget/git/issues/1680
  [3] https://discord.com/channels/1042895022950994071/1213052410281467906

I don't know if it is the polite thing to ask you to please refer to
the information I linked, or if I should duplicate the information
here? The short version is that I need to be able to distinguish
between config entries made by automation (like scripts, or Ansible
in my particular case) and those made by a human.

>> git config --comment "I changed this. --A. Script" \
>>   --add safe.directory /home/alice/somerepo.git
>
> If you are illustrating a sample input, please also explain what
> output it produces. What do the resulting lines in the config file
> look like after you run this command?

The result of running the above command looks as follows:

  [safe]
	directory = /home/alice/somerepo.git #I changed this. --A. Script

> Why are you adding "# comment" to your config file? Who reads these
> comments, with what tool, and for what purpose?

I mentioned human-readable comments in the patch, and humans are
indeed the intended audience. If a human finds changes made to a Git
config file, and a comment states that the modification was e.g. made
by automation, it adds beneficial information for said human. I can for
example create a comment with a URL pointing to a website providing
additional explanations.

> Now how do we find out about this comment? "git config -l" would
> not give us that. Are we expected to look at it in our editor or
> pager?

Yes. I routinely use cat/vim to inspect/modify my Git config
files. They are suitable for human consumption, after all. Also,
comments can already be manually added in creative ways, and are
ignored when Git reads config data. Comments being read only by
humans is pretty much their whole point, in my opinion.

> Can we come up with a code that reliably decides when to remove the
> first comment we see above?

Your examples about difficulties removing comments hinge on there
being multiline comments, as far as I can tell. My patch only supports
single-line comments, and only as a suffix to newly added key-value
pairs. This is a deliberate design choice.

> The above is an illustration of what I want to see the author, who
> comes up with this "wouldn't it be wonderful to teach git-config to
> add comment?" idea, thinks through. The first patch might be to
> just add a comment when a variable=value is added, but we want to
> see the vision of how the whole end-user experience should look
> like, in which this first small piece will fit.

I don't have a greater vision for comments. Their use is to provide
information for humans, no more, no less. There is also no idea of a
user experience beyond pager/editor in my mind. The patch addresses
specific needs of mine, and Johannes suggested it as a new feature,
and that's all the motivation behind it.

> And the amount of the change required for that tiny bit of
> "improvement" (if we can call it, which is dubious) does not seem
> worth it.

As I mentioned in my PR, it also does not seem elegant to me to modify
so many files. Alas, C does not offer expanding function signatures by
adding parameters with default values, like Python does. Adding a new
function like, perhaps, git_config_set_in_file_gently_with_comment()
could be a remedy?

-Ralph




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux