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@xxxxxxxxx/ 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@xxxxxxxxxxxxxxxxxxxxxx/ 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@xxxxxxxxxxxxxxxxxxxxxx/ 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
