Re: [PATCH v2 1/1] config: move documentation to config.h

2019-10-22 Thread Heba Waly
On Wed, Oct 23, 2019 at 9:59 AM Emily Shaffer  wrote:
>
> On Tue, Oct 22, 2019 at 07:05:06AM +, Heba Waly via GitGitGadget wrote:
> > From: Heba Waly 
> >
> > Move the documentation from Documentation/technical/api-config.txt into
> > config.h
>
> This is still a little thin for what we usually want from commit
> messages. Try to imagine that five years from now, you find this commit
> by running `git blame` on config.h and then examining the commit which
> introduced all these comments with `git show ` - what would
> you want to know?
>
> Typically we want to know "why" the change was made, because the diff
> shows "what". We can see from the diff that you're moving comments from
> A to B, but if you explain why you did so (not "because my Outreachy
> mentor told me to" ;) but "because it is useful to see usage information
> next to code" or "this is best practice as described by blah blah") - I
> wouldn't be able to know that reasoning just from looking at your diff.
Ok, got it.

>
> > diff --git a/config.h b/config.h
> > index f0ed464004..02f78ffc2b 100644
> > --- a/config.h
> > +++ b/config.h
> > @@ -4,6 +4,23 @@
> >  #include "hashmap.h"
> >  #include "string-list.h"
> >
> > +
> > +/**
> > + * The config API gives callers a way to access Git configuration files
> > + * (and files which have the same syntax). See linkgit:git-config[1] for a
>
> Ah, here's another place where the Asciidoc link isn't going to do
> anything anymore.
yep!
> Otherwise I didn't still see anything jumping out. When the commit
> message is cleaned up I'm ready to add my Reviewed-by line.
great!

>  - Emily

Thanks :)


Re: [PATCH v2 1/1] config: move documentation to config.h

2019-10-22 Thread Junio C Hamano
Emily Shaffer  writes:

>> ...
>> +/**
>> + * The config API gives callers a way to access Git configuration files
>> + * (and files which have the same syntax). See linkgit:git-config[1] for a
>
> Ah, here's another place where the Asciidoc link isn't going to do
> anything anymore.
>
> Otherwise I didn't still see anything jumping out. When the commit
> message is cleaned up I'm ready to add my Reviewed-by line.

Thanks.  Your review(s) have been quite sensible and helpful.




Re: [PATCH v2 1/1] config: move documentation to config.h

2019-10-22 Thread Emily Shaffer
On Tue, Oct 22, 2019 at 07:05:06AM +, Heba Waly via GitGitGadget wrote:
> From: Heba Waly 
> 
> Move the documentation from Documentation/technical/api-config.txt into
> config.h

This is still a little thin for what we usually want from commit
messages. Try to imagine that five years from now, you find this commit
by running `git blame` on config.h and then examining the commit which
introduced all these comments with `git show ` - what would
you want to know?

Typically we want to know "why" the change was made, because the diff
shows "what". We can see from the diff that you're moving comments from
A to B, but if you explain why you did so (not "because my Outreachy
mentor told me to" ;) but "because it is useful to see usage information
next to code" or "this is best practice as described by blah blah") - I
wouldn't be able to know that reasoning just from looking at your diff.


> diff --git a/config.h b/config.h
> index f0ed464004..02f78ffc2b 100644
> --- a/config.h
> +++ b/config.h
> @@ -4,6 +4,23 @@
>  #include "hashmap.h"
>  #include "string-list.h"
>  
> +
> +/**
> + * The config API gives callers a way to access Git configuration files
> + * (and files which have the same syntax). See linkgit:git-config[1] for a

Ah, here's another place where the Asciidoc link isn't going to do
anything anymore.

Otherwise I didn't still see anything jumping out. When the commit
message is cleaned up I'm ready to add my Reviewed-by line.

 - Emily


[PATCH v2 1/1] config: move documentation to config.h

2019-10-22 Thread Heba Waly via GitGitGadget
From: Heba Waly 

Move the documentation from Documentation/technical/api-config.txt into
config.h
Signed-off-by: Heba Waly 
---
 Documentation/technical/api-config.txt | 319 ---
 config.h   | 336 +
 2 files changed, 336 insertions(+), 319 deletions(-)
 delete mode 100644 Documentation/technical/api-config.txt

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
deleted file mode 100644
index 7d20716c32..00
--- a/Documentation/technical/api-config.txt
+++ /dev/null
@@ -1,319 +0,0 @@
-config API
-==
-
-The config API gives callers a way to access Git configuration files
-(and files which have the same syntax). See linkgit:git-config[1] for a
-discussion of the config file syntax.
-
-General Usage
--
-
-Config files are parsed linearly, and each variable found is passed to a
-caller-provided callback function. The callback function is responsible
-for any actions to be taken on the config option, and is free to ignore
-some options. It is not uncommon for the configuration to be parsed
-several times during the run of a Git program, with different callbacks
-picking out different variables useful to themselves.
-
-A config callback function takes three parameters:
-
-- the name of the parsed variable. This is in canonical "flat" form: the
-  section, subsection, and variable segments will be separated by dots,
-  and the section and variable segments will be all lowercase. E.g.,
-  `core.ignorecase`, `diff.SomeType.textconv`.
-
-- the value of the found variable, as a string. If the variable had no
-  value specified, the value will be NULL (typically this means it
-  should be interpreted as boolean true).
-
-- a void pointer passed in by the caller of the config API; this can
-  contain callback-specific data
-
-A config callback should return 0 for success, or -1 if the variable
-could not be parsed properly.
-
-Basic Config Querying
--
-
-Most programs will simply want to look up variables in all config files
-that Git knows about, using the normal precedence rules. To do this,
-call `git_config` with a callback function and void data pointer.
-
-`git_config` will read all config sources in order of increasing
-priority. Thus a callback should typically overwrite previously-seen
-entries with new ones (e.g., if both the user-wide `~/.gitconfig` and
-repo-specific `.git/config` contain `color.ui`, the config machinery
-will first feed the user-wide one to the callback, and then the
-repo-specific one; by overwriting, the higher-priority repo-specific
-value is left at the end).
-
-The `config_with_options` function lets the caller examine config
-while adjusting some of the default behavior of `git_config`. It should
-almost never be used by "regular" Git code that is looking up
-configuration variables. It is intended for advanced callers like
-`git-config`, which are intentionally tweaking the normal config-lookup
-process. It takes two extra parameters:
-
-`config_source`::
-If this parameter is non-NULL, it specifies the source to parse for
-configuration, rather than looking in the usual files. See `struct
-git_config_source` in `config.h` for details. Regular `git_config` defaults
-to `NULL`.
-
-`opts`::
-Specify options to adjust the behavior of parsing config files. See `struct
-config_options` in `config.h` for details. As an example: regular `git_config`
-sets `opts.respect_includes` to `1` by default.
-
-Reading Specific Files
---
-
-To read a specific file in git-config format, use
-`git_config_from_file`. This takes the same callback and data parameters
-as `git_config`.
-
-Querying For Specific Variables

-
-For programs wanting to query for specific variables in a non-callback
-manner, the config API provides two functions `git_config_get_value`
-and `git_config_get_value_multi`. They both read values from an internal
-cache generated previously from reading the config files.
-
-`int git_config_get_value(const char *key, const char **value)`::
-
-   Finds the highest-priority value for the configuration variable `key`,
-   stores the pointer to it in `value` and returns 0. When the
-   configuration variable `key` is not found, returns 1 without touching
-   `value`. The caller should not free or modify `value`, as it is owned
-   by the cache.
-
-`const struct string_list *git_config_get_value_multi(const char *key)`::
-
-   Finds and returns the value list, sorted in order of increasing priority
-   for the configuration variable `key`. When the configuration variable
-   `key` is not found, returns NULL. The caller should not free or modify
-   the returned pointer, as it is owned by the cache.
-
-`void git_config_clear(void)`::
-
-   Resets and invalidates the config cache.
-
-The config API also provides type specific API functions