[PATCH v3 0/3] submodule: port subcommand add from shell to C

[Shourya Shukla <periperidip@gmail.com>]

Greetings,

This is the v3 of the patch series with the same title. You may view
the v2 here:
https://lore.kernel.org/git/20201007074538.25891-1-shouryashukla.oo@gmail.com/

I have applied almost all of the changes asked before, except a few
which confused me a little. It would be great if I could get some help
about them:

    1. In this mail: https://lore.kernel.org/git/xmqqo8ldznjx.fsf@gitster.c.googlers.com/
       Junio asked me to accomodate for a merge in progress since
       'cache_pos < 0' does not necessarily mean that the path exists.
       I was wondering which function would be the most appropriate for
       the if-statements:
            if (!force) {
		        if (cache_pos >= 0 || dir_in_cache)
            }
       I was thinking of going with 'read_cache_unmerged()'. I thought
       of this by seeing what is triggered in case of a merge conflict
       and cam across this. What is your opinion on this?

    2. In this mail: https://lore.kernel.org/git/xmqqimbky6st.fsf@gitster.c.googlers.com/
       In this section:
            /* strip trailing '/' */
	        if (is_dir_sep(sm_path[strlen(sm_path) -1]))
		        sm_path[strlen(sm_path) - 1] = '\0';

       Junio makes a reasonable argument that we need to make sure that
       multiple trailing slashes are eliminated but my code only takes
       care of a single trailing slash. I was looking into the code of
       'normalize_path_copy()' and saw that the function it essentially
       calls: 'normalize_path_copy_len()' does not perform anything on
       the trailing slashes and this behaviour is mentioned as a
       NEEDSWORK.

       I was thinking of correcting the above function instead of
       putting in an extra loop. Is this feasible?

    3. In the following segment:
        /*
         * NEEDSWORK: In a multi-working-tree world, this needs to be
         * set in the per-worktree config.
         */
        if (!git_config_get_string("submodule.active", &var) && var) {

        There was a comment: "What if this were a valueless true
        ("[submodule] active\n" without "= true")?  Wouldn't get_string()
        fail?"

        I was under the impression that even if the above failed, it
        will not really affect the big picture since at the we will set
        'submodule.name.active" as true irrespective of the above value.
        Is this correct?

Feedback and reviews are appreciated.

Regards,
Shourya Shukla

Shourya Shukla (3):
  dir: change the scope of function 'directory_exists_in_index()'
  submodule: port submodule subcommand 'add' from shell to C
  t7400: add test to check 'submodule add' for tracked paths

 builtin/submodule--helper.c | 410 +++++++++++++++++++++++++++++++++++-
 dir.c                       |  30 ++-
 dir.h                       |   9 +
 git-submodule.sh            | 161 +-------------
 t/t7400-submodule-basic.sh  |  13 +-
 5 files changed, 443 insertions(+), 180 deletions(-)

-- 
2.25.1