Re: [PATCH RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Fabian Ruch writes: > On 07/08/2014 10:45 PM, Junio C Hamano wrote: >> Fabian Ruch writes: >>> Fabian Ruch (19): >>> rebase -i: Failed reword prints redundant error message >>> rebase -i: reword complains about empty commit despite --keep-empty >>> rebase -i: reword executes pre-commit hook on interim commit >>> rebase -i: Teach do_pick the option --edit >>> rebase -i: Implement reword in terms of do_pick >>> rebase -i: Stop on root commits with empty log messages >>> rebase -i: The replay of root commits is not shown with --verbose >>> rebase -i: Root commits are replayed with an unnecessary option >>> rebase -i: Commit only once when rewriting picks >>> rebase -i: Do not die in do_pick >>> rebase -i: Teach do_pick the option --amend >>> rebase -i: Teach do_pick the option --file >>> rebase -i: Prepare for squash in terms of do_pick --amend >>> rebase -i: Implement squash in terms of do_pick >>> rebase -i: Explicitly distinguish replay commands and exec tasks >>> rebase -i: Parse to-do list command line options >>> rebase -i: Teach do_pick the option --reset-author >>> rebase -i: Teach do_pick the option --signoff >>> rebase -i: Enable options --signoff, --reset-author for pick, reword >> >> After "rebase -i:", some begin with lowercase and many begin with >> capital, which makes the short-log output look distracting. > > The ones that begin with lower-case letters are the ones that begin with > the command name "reword". All first lines are typed in lower case now. You could spell it 'reword' (with the quotes), which also disambiguates the command from the verb. -- Thomas Rast t...@thomasrast.ch -- 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 RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
On 07/08/2014 10:45 PM, Junio C Hamano wrote: > Fabian Ruch writes: >> Fabian Ruch (19): >> rebase -i: Failed reword prints redundant error message >> rebase -i: reword complains about empty commit despite --keep-empty >> rebase -i: reword executes pre-commit hook on interim commit >> rebase -i: Teach do_pick the option --edit >> rebase -i: Implement reword in terms of do_pick >> rebase -i: Stop on root commits with empty log messages >> rebase -i: The replay of root commits is not shown with --verbose >> rebase -i: Root commits are replayed with an unnecessary option >> rebase -i: Commit only once when rewriting picks >> rebase -i: Do not die in do_pick >> rebase -i: Teach do_pick the option --amend >> rebase -i: Teach do_pick the option --file >> rebase -i: Prepare for squash in terms of do_pick --amend >> rebase -i: Implement squash in terms of do_pick >> rebase -i: Explicitly distinguish replay commands and exec tasks >> rebase -i: Parse to-do list command line options >> rebase -i: Teach do_pick the option --reset-author >> rebase -i: Teach do_pick the option --signoff >> rebase -i: Enable options --signoff, --reset-author for pick, reword > > After "rebase -i:", some begin with lowercase and many begin with > capital, which makes the short-log output look distracting. The ones that begin with lower-case letters are the ones that begin with the command name "reword". All first lines are typed in lower case now. Sorry for the noise. Fabian -- 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 RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Fabian Ruch writes: > Fabian Ruch (19): > rebase -i: Failed reword prints redundant error message > rebase -i: reword complains about empty commit despite --keep-empty > rebase -i: reword executes pre-commit hook on interim commit > rebase -i: Teach do_pick the option --edit > rebase -i: Implement reword in terms of do_pick > rebase -i: Stop on root commits with empty log messages > rebase -i: The replay of root commits is not shown with --verbose > rebase -i: Root commits are replayed with an unnecessary option > rebase -i: Commit only once when rewriting picks > rebase -i: Do not die in do_pick > rebase -i: Teach do_pick the option --amend > rebase -i: Teach do_pick the option --file > rebase -i: Prepare for squash in terms of do_pick --amend > rebase -i: Implement squash in terms of do_pick > rebase -i: Explicitly distinguish replay commands and exec tasks > rebase -i: Parse to-do list command line options > rebase -i: Teach do_pick the option --reset-author > rebase -i: Teach do_pick the option --signoff > rebase -i: Enable options --signoff, --reset-author for pick, reword After "rebase -i:", some begin with lowercase and many begin with capital, which makes the short-log output look distracting. -- 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 RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
On 07/02/2014 07:47 PM, Fabian Ruch wrote: > [...] > 2. Add user options to main commands > > Enable options --signoff, --reset-author for pick, reword (19/19) > > The last stage was added in this reroll. It enables the parsing of > line options for to-do list commands, which is still restricted to > options without arguments because it is unclear how spaces can be > parsed as characters rather than separators where needed. For > instance, if we were to support > > pick --author="A U Thor" fa1afe1 Some changes > > then read(1) would hand us the tokens `--author="A`, `U` and `Thor"` > instead of `--author=A U Thor`, which we would want to relay to > `do_pick`. Interpreting the shell quoting would help. However, > eval(1) seems to disqualify itself as an interpreter because the > to-do list entry could potentially contain any shell command line. > This could be both a security and a usability issue. For this reason, > the patch series still hasn't graduated from being RFC. > [...] It is not required that a patch series solves all of the problems in the universe. If these patches implements some useful features robustly, and if there is no reason to expect that future enhancements will require the user interface to be changed in a backwards-compatible way, then there is no reason that this patch series has to be held as an RFC hostage to some hypothetical future. Michael -- Michael Haggerty mhag...@alum.mit.edu -- 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 RFC v2 00/19] Enable options --signoff, --reset-author for pick, reword
Hi, this reroll applies the comments from Eric, Junio and Michael. In particular, * It turned out that `pick_one` does not need option handling at all and the option-like argument `-n` determines whether `pick_one` or `do_pick` creates the replay commit. The latter happens if the task wants to rewrite the commit being picked (f.i., for the purpose of editing the log message or resetting the authorship). `do_pick` still seems to require a flexible parsing of options, i.e. a relatively expensive loop, since it receives the whitelisted user-supplied options. Unsupported and unknown options are treated as an "unknown command" error. * The `do_pick` options are documented in the same order they are listed in the function signature. Moreover, it is mentioned which options rewrite commits being picked. * The test cases output differing actual values as changes to the expected values and not the other way around. Moreover, the failed rebases are not cleaned up until the respective test succeeds. Two stages (and two sub-stages) can be identified in the rerolled patch series: 1. Route primary to-do list commands through `do_pick` a. Implement reword in terms of do_pick (5/19) b. Implement squash in terms of do_pick (14/19) 2. Add user options to main commands Enable options --signoff, --reset-author for pick, reword (19/19) The last stage was added in this reroll. It enables the parsing of line options for to-do list commands, which is still restricted to options without arguments because it is unclear how spaces can be parsed as characters rather than separators where needed. For instance, if we were to support pick --author="A U Thor" fa1afe1 Some changes then read(1) would hand us the tokens `--author="A`, `U` and `Thor"` instead of `--author=A U Thor`, which we would want to relay to `do_pick`. Interpreting the shell quoting would help. However, eval(1) seems to disqualify itself as an interpreter because the to-do list entry could potentially contain any shell command line. This could be both a security and a usability issue. For this reason, the patch series still hasn't graduated from being RFC. Fabian Fabian Ruch (19): rebase -i: Failed reword prints redundant error message rebase -i: reword complains about empty commit despite --keep-empty rebase -i: reword executes pre-commit hook on interim commit rebase -i: Teach do_pick the option --edit rebase -i: Implement reword in terms of do_pick rebase -i: Stop on root commits with empty log messages rebase -i: The replay of root commits is not shown with --verbose rebase -i: Root commits are replayed with an unnecessary option rebase -i: Commit only once when rewriting picks rebase -i: Do not die in do_pick rebase -i: Teach do_pick the option --amend rebase -i: Teach do_pick the option --file rebase -i: Prepare for squash in terms of do_pick --amend rebase -i: Implement squash in terms of do_pick rebase -i: Explicitly distinguish replay commands and exec tasks rebase -i: Parse to-do list command line options rebase -i: Teach do_pick the option --reset-author rebase -i: Teach do_pick the option --signoff rebase -i: Enable options --signoff, --reset-author for pick, reword git-rebase--interactive.sh| 277 ++ t/t3404-rebase-interactive.sh | 8 ++ t/t3412-rebase-root.sh| 39 ++ 3 files changed, 273 insertions(+), 51 deletions(-) -- 2.0.0 -- 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
