Re: [PATCH] config: Add documentation for writing config files

2014-06-03 Thread Tanay Abhra
On 06/02/2014 12:37 PM, Matthieu Moy wrote:
> Tanay Abhra  writes:
> 
>> Signed-off-by: Tanay Abhra 
>> ---
>>  Documentation/technical/api-config.txt | 31 ++-
>>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> Even though the reason to replace a TODO with real stuff is rather
> straigthforward, the reader appriates a short commit message (ideally
> pointing to the commit introducing the TODO). My first thought reading
> the patch was "wtf, is that a patch in the middle of a series, where
> does this TODO come from" ;-).

Ok, I will send a new patch with the updated commit message. .

>> +In the end they both all call `git_config_set_multivar_in_file` which takes
>> +four parameters:
> 
> Do we really want to document this as part of the config API? i.e. does
> a normal user of the API want to know about this? My understanding is
> that git_config_set_multivar_in_file is essentially a private function,
> and then the best place to document is with comments near the function
> definition (it already has such comment).

Sorry, but I don't think so. In cache.h, the following functions have been 
provided
as externs,
git_config_set_in_file()
git_config_set()
git_config_set_multivar()
git_config_set_multivar_in_file()
extern int git_config_rename_section()
extern int git_config_rename_section_in_file()

Thus, they seem to be the part of the API. In most of the technical API docs I 
have
read there is at least a description of all parameters of the main function 
also,
relevant data structures if any.
Also a certain amount of redundancy about details is allowed.

A good example is api-hashmap.txt which provides detailed descriptions of each
function and data structure which was very much helpful to me.

Reverse is api-string-list.txt which was useless to me, so I had to go through
the whole code to understand how to use it.

Thanks for the review.



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] config: Add documentation for writing config files

2014-06-02 Thread Matthieu Moy
Tanay Abhra  writes:

> Signed-off-by: Tanay Abhra 
> ---
>  Documentation/technical/api-config.txt | 31 ++-
>  1 file changed, 30 insertions(+), 1 deletion(-)

Even though the reason to replace a TODO with real stuff is rather
straigthforward, the reader appriates a short commit message (ideally
pointing to the commit introducing the TODO). My first thought reading
the patch was "wtf, is that a patch in the middle of a series, where
does this TODO come from" ;-).

> diff --git a/Documentation/technical/api-config.txt 
> b/Documentation/technical/api-config.txt
> index 230b3a0..df08385 100644
> --- a/Documentation/technical/api-config.txt
> +++ b/Documentation/technical/api-config.txt
> @@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t 
> fn, void *data)
>  Writing Config Files
>  
>  
> -TODO
> +Git gives multiple entry points in the Config API to write config values to
> +files namely `git_config_set_in_file` and `git_config_set`, which write to
> +a specific config file or to `.git/config` respectively. They both take a
> +key/value pair as parameter.

Sounds good.

> +In the end they both all call `git_config_set_multivar_in_file` which takes
> +four parameters:

Do we really want to document this as part of the config API? i.e. does
a normal user of the API want to know about this? My understanding is
that git_config_set_multivar_in_file is essentially a private function,
and then the best place to document is with comments near the function
definition (it already has such comment).

Your text is easier to understand and more detailed, but I would
personnally prefer improving the in-code comment (possibly just leaving
a mention of git_config_set_multivar_in_file and pointing the reader to
the code for details).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] config: Add documentation for writing config files

2014-06-02 Thread Tanay Abhra
Signed-off-by: Tanay Abhra 
---
 Documentation/technical/api-config.txt | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/technical/api-config.txt 
b/Documentation/technical/api-config.txt
index 230b3a0..df08385 100644
--- a/Documentation/technical/api-config.txt
+++ b/Documentation/technical/api-config.txt
@@ -137,4 +137,33 @@ int read_file_with_include(const char *file, config_fn_t 
fn, void *data)
 Writing Config Files
 
 
-TODO
+Git gives multiple entry points in the Config API to write config values to
+files namely `git_config_set_in_file` and `git_config_set`, which write to
+a specific config file or to `.git/config` respectively. They both take a
+key/value pair as parameter.
+In the end they both all call `git_config_set_multivar_in_file` which takes
+four parameters:
+
+- the name of the file, as a string, to which key/value pairs will be written.
+
+- the name of key, as a string. 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 variable, as a string. If value is equal to NULL, it will
+  remove the matching key from the config file.
+
+- the value regex, as a string. It will disregard key/value pairs where value
+  does not match.
+
+- a multi_replace value, as an int. If value is equal to zero, nothing or only
+  one matching key/value is replaced, else all matching key/values (regardless
+  how many) are removed, before the new pair is written.
+
+It returns 0 on success.
+
+Also, there are functions `git_config_rename_section` and
+`git_config_rename_section_in_file` with parameters `old_name` and `new_name`
+for renaming or removing sections in the config files. If NULL is passed
+through `new_name` parameter, the section will be removed from the config file.
-- 
1.9.0.GIT

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html