# [GSoC] Proposal 6 messages from 2016-03-24 to 2016-03-25. Participants: XZS, Junio C Hamano, Dominik Fischer, Christian Couder. Thread: https://gitlist.dev/t/41817 ## XZS, 2016-03-24 21:09 Subject: [GSoC] Proposal Message-ID: <1458853787-29807-1-git-send-email-d.f.fischer@web.de> URL: https://gitlist.dev/e/1458853787-29807-1-git-send-email-d.f.fischer%40web.de ``` Greetings, I hope it is not yet too late to jump on the Summer of Code bandwagon. I would appreciate comments on my application [1] and my microproject contribution, which will follow this mail as a reply. My proposal mostly stems from what was noted under "convert scripts to builtins" and "git rebase improvements" in the ideas page. Both list no mentor, so please let me know if you know anyone who should be mentioned in CC. Regards, XZS. [1]: https://docs.google.com/document/d/1-BV-s5VUGTvBlcVDeo6tVqQO5D1hqeQDqaf37iYuIfU/edit?usp=sharing -- 2.7.4 ``` ## XZS, 2016-03-24 21:09 Subject: [PATCH/GSoC] add a add.patch config variable Message-ID: <1458853787-29807-2-git-send-email-d.f.fischer@web.de> URL: https://gitlist.dev/e/1458853787-29807-2-git-send-email-d.f.fischer%40web.de In-Reply-To: <1458853787-29807-1-git-send-email-d.f.fischer@web.de> ``` Users may like to review their changes when staging by default. It is also a convenient safety feature for beginners nudging them to have a second look at their changes when composing a commit. To this end, the config variable allows to have git-add to always act like -p was passed. Signed-off-by: XZS --- The list of microproject suggestions for the Summer of Code 2016 proposed to "add configuration options for some commonly used command-line options", so I added a configuraion variable for an option I use all the time. Documentation/config.txt | 5 +++++ Documentation/git-add.txt | 3 +++ builtin/add.c | 3 +++ contrib/completion/git-completion.bash | 1 + t/t3701-add-interactive.sh | 9 +++++++++ 5 files changed, 21 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 2cd6bdd..614c599 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -752,6 +752,11 @@ add.ignore-errors (deprecated):: as it does not follow the usual naming convention for configuration variables. +add.patch:: + Configures 'git add' to always interactively choose hunks, hinting the + user to review changes before staging. This is equivalent to adding the + '--patch' option to linkgit:git-add[1]. + alias.*:: Command aliases for the linkgit:git[1] command wrapper - e.g. after defining "alias.last = cat-file commit HEAD", the invocation diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 6a96a66..cdb6663 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -92,6 +92,9 @@ OPTIONS This effectively runs `add --interactive`, but bypasses the initial command menu and directly jumps to the `patch` subcommand. See ``Interactive mode'' for details. ++ +The configuration variable `add.patch` can be set to true to make +this the default behaviour. -e:: --edit:: diff --git a/builtin/add.c b/builtin/add.c index 145f06e..04f8b5e 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -272,6 +272,9 @@ static int add_config(const char *var, const char *value, void *cb) !strcmp(var, "add.ignore-errors")) { ignore_add_errors = git_config_bool(var, value); return 0; + } else if (!strcmp(var, "add.patch")) { + patch_interactive = git_config_bool(var, value); + return 0; } return git_default_config(var, value, cb); } diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e3918c8..597d20f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1969,6 +1969,7 @@ _git_config () esac __gitcomp " add.ignoreErrors + add.patch advice.commitBeforeMerge advice.detachedHead advice.implicitIdentity diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index deae948..25e4b2e 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -380,4 +380,13 @@ test_expect_success 'patch mode ignores unmerged entries' ' test_cmp expected diff ' +test_expect_success 'patch mode can be activated per option' ' + git config add.patch true && + git reset --hard && + echo change >test && + echo y | git add -p > output && + cat output && + grep "Stage this hunk \[y,n,q,a,d,/,e,?\]?" output +' + test_done -- 2.7.4 ``` ## Junio C Hamano, 2016-03-24 21:20 Subject: Re: [PATCH/GSoC] add a add.patch config variable Message-ID: URL: https://gitlist.dev/e/xmqqvb4b7eu9.fsf%40gitster.mtv.corp.google.com In-Reply-To: <1458853787-29807-2-git-send-email-d.f.fischer@web.de> ``` XZS writes: > Users may like to review their changes when staging by default. It is > also a convenient safety feature for beginners nudging them to have a > second look at their changes when composing a commit. > > To this end, the config variable allows to have git-add to always act > like -p was passed. Now with such a configuration in her ~/.gitconfig, how would she ever run the normal "git add", which perhaps is invoked by one of the scripts she regularly uses? E.g. "git mergetool"? ``` ## Dominik Fischer, 2016-03-25 00:43 Subject: Re: [PATCH/GSoC] add a add.patch config variable Message-ID: <56F489C3.9050400@web.de> URL: https://gitlist.dev/e/56F489C3.9050400%40web.de In-Reply-To: ``` Am 24.03.2016 um 22:20 schrieb Junio C Hamano: > XZS writes: >> Users may like to review their changes when staging by default. It is >> also a convenient safety feature for beginners nudging them to have a >> second look at their changes when composing a commit. >> >> To this end, the config variable allows to have git-add to always act >> like -p was passed. > > Now with such a configuration in her ~/.gitconfig, how would she > ever run the normal "git add", which perhaps is invoked by one of > the scripts she regularly uses? E.g. "git mergetool"? As the configuration variable can be overwritten by a command line option, I am tempted to amend this by replacing all occurrences of "git add" in other scripts with "git add --no-patch" to ensure the expected behavior. But this would introduce changes into a vast number of points in the code. Apart from that, I suspect other options may have the same reason not to be available as config variables. It would be much better if git-add could ignore the variable when called internally, only taking it into account when used as an entrypoint. Is there already a mechanism in place to determine if git was called by git? If not, I could implement one through an environment variable that counts up on each git invocation, essentially providing a git recursion counter. ``` ## Junio C Hamano, 2016-03-25 06:30 Subject: Re: [PATCH/GSoC] add a add.patch config variable Message-ID: URL: https://gitlist.dev/e/xmqqio0b5ash.fsf%40gitster.mtv.corp.google.com In-Reply-To: <56F489C3.9050400@web.de> ``` Dominik Fischer writes: > As the configuration variable can be overwritten by a command line > option, I am tempted to amend this by replacing all occurrences of > "git add" in other scripts with "git add --no-patch" to ensure the > expected behavior. Don't even think about it--you won't know all scripts you have to fix like that, as a lot more invocations of "git add" appear in end user's scripts that we have never seen. ``` ## Christian Couder, 2016-03-25 07:01 Subject: Re: [PATCH/GSoC] add a add.patch config variable Message-ID: URL: https://gitlist.dev/e/CAP8UFD0KZk1nTgR8gdbedsAuw2ztWZDWri6EkeEkf8FhwS9yag%40mail.gmail.com In-Reply-To: <1458853787-29807-2-git-send-email-d.f.fischer@web.de> ``` On Thu, Mar 24, 2016 at 10:09 PM, XZS wrote: > > diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh > index deae948..25e4b2e 100755 > --- a/t/t3701-add-interactive.sh > +++ b/t/t3701-add-interactive.sh > @@ -380,4 +380,13 @@ test_expect_success 'patch mode ignores unmerged entries' ' > test_cmp expected diff > ' > > +test_expect_success 'patch mode can be activated per option' ' > + git config add.patch true && > + git reset --hard && > + echo change >test && > + echo y | git add -p > output && Why are you using "-p" if you want to test that the config option works? > + cat output && > + grep "Stage this hunk \[y,n,q,a,d,/,e,?\]?" output > +' > + > test_done ```