Re:[GSOC] [PROPOSAL v3] Draft of proposal for "Unify ref-filter formats with other pretty formats"

["Zhang Yi" <18994118902@163.com>]

Hi, I have done some preparatory work. I read most part of the patch
"ref-filter: add new "signature" atom"[1] but the part of the test script is
left.
I plan to read details about test scripts if the previous work of the patch is
well down.
* My questions
** Are there more aspects I need to focus when reading other's
patches?
** Are there other patches necessary to read?


Thanks for all suggestions.
Below is what I grasp from ref-filter: add new "signature" atom patches. I will
put it on my blog site later.


* v1
** The goal of v1
Add "signature" atom with `grade`,`signer`, `key`, `fingerprint`,
`primarykeyfingerprint`, `trustlevel` as arguments.
To get constitutes for %GG, %G?, %GS, %GK, %GF, %GP, and %GT pretty formats.
** The approach
*** enum
atom_type; ATOM_SIGNATURE: used as the index of valid_atom array.
*** struct
used_atom; struct {...} signature: add signature atom and its options.
valid_atom ; [ATOM_SIGNATURE]: seems build a map between "signature" and
function "signature_atom_parser".
*** function
**** add
static int signature_atom_parser: Set "signature.option" according to args.
static void grab_signature: Set the result string in atom_value.value for
signature options. There are more details need to understand.
**** modify
static void grab_values: add grab_signature in case OBJ_COMMIT.
*** document
*** test scripts
** Others' comments on v1
*** Junio C Hamano
1. Lack motivation.
2. Need to deal with signed tag.
3. Improve the style of "if else".
4. Rename grab_signature to grab_commit_signature. This make easier for future
developers to understand and add "grab_tag_signature".
5. Add space between cast and value.
6. Check used-atom to make sure there is need to do signature processing.
7. A call to check_commit_signature() should have a matching call to
signature_check_clear().
*** Jeff King
1. Add an annotation for the unused parameter, which is necessary when
-Wunused-parameter is on.
2. In signature_atom_parser, return err_bad_arg for wrong arg. This make the
error message match the others.
* v2 seems meet some mistake about message ID
1. More detailed commit message.
2. Add parse_signature_option(), which return value for struct signature.option
or -1 when get wrong arg. This function is used in grab_signature() and
signature_atom_parser().
3. Modify signature_atom_parser(). Use parse_signature_option() to get opt and
return err_bad_arg for wrong arg.
4. Modify grab_signature(). Use parse_signature_option() to check name in
used_atom and use check_commit_signature() with signature_check_clear() only
once.
5. Add test for bare signature atom(%(signature)).
* v3 and v4
1. Remove test for bare signature atom because the result of the test is not
same on different platform.
** Junio C Hamano's comments
Avoid calling check_commit_signature() when no %(signature) atoms is used.
* v5
1. Add a signature_checked flag to avoid calling check_commit_signature()
unnecessarily.
2. Fix the test for bare signature atom(%(signature)) about trustdb.
** Junio C Hamano's comments
A question about differences between versions of GPG. It seems no relationship
with my work.
* View these patches a whole
** The general direction
1. Register a new signature and its options in enum atom_type and struct used
atom.
2. Define a foo_atom_parser() to convert string arg to int option.
3. Bind atom with its foo_atom_parser() in valid_atom.
4. Define a grab_foo() to set string s in struct atom_value according to option.
5. Insert the grab_foo() into grab_values() function.
** ls it possible to use the similar approach
Yes. I can follow the general idea of this patch. Of course, I need to finetune
the details according to the atom I work on.
* Else
check_commit_signature() is defined in commit.c.
What is GPG?
https://gnupg.org/
GnuPG allows you to encrypt and sign your data and communications.


[1] https://public-inbox.org/git/pull.1452.git.1672102523902.gitgitgadget@gmail.com/