Re: [PATCH 19/22] sequencer: support cleaning up commit messages
- From
Jakub Narębski <jnareb@gmail.com>
- Date
- Sep 1, 2016, 10:31 UTC
- Message-ID
- <9f2cbc74-3e4d-e7f1-d2dd-e85e0b9168b1@gmail.com>
- In-Reply-To
- <eeb3d11d235a0220a9a125a21d1b09a73d2c61dc.1472457609.git.johannes.schindelin@gmx.de>
Hello Johannes,
W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:
Show 10 quoted lines
> The sequencer_commit() function already knows how to amend commits, and > with this new option, it can also clean up commit messages (i.e. strip > out commented lines). This is needed to implement rebase -i's 'fixup' > and 'squash' commands as sequencer commands. > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > sequencer.c | 10 +++++++--- > sequencer.h | 3 ++- > 2 files changed, 9 insertions(+), 4 deletions(-)
This looks like nice little piece of enhancement, building scaffolding for sequencer-izing interactive rebase bit by bit.
Show 12 quoted lines
> > diff --git a/sequencer.c b/sequencer.c > index 20f7590..5ec956f 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -478,7 +478,8 @@ static char **read_author_script(void) > * (except, of course, while running an interactive rebase). > */ > int sequencer_commit(const char *defmsg, struct replay_opts *opts, > - int allow_empty, int edit, int amend) > + int allow_empty, int edit, int amend, > + int cleanup_commit_message)
All right, though it slowly begins coming close to the threshold where using bitfield flags would be sensible.
Show 9 quoted lines
> {
> char **env = NULL;
> struct argv_array array;
> @@ -515,9 +516,12 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> argv_array_push(&array, "-s");
> if (defmsg)
> argv_array_pushl(&array, "-F", defmsg, NULL);
> + if (cleanup_commit_message)
> + argv_array_push(&array, "--cleanup=strip");Good.
> if (edit) > argv_array_push(&array, "-e"); > - else if (!opts->signoff && !opts->record_origin && > + else if (!cleanup_commit_message &&
All right, explicit cleanup=strip overrides "commit.cleanup" config, and turns off passing commit verbatim (incompatible with stripping)...
> + !opts->signoff && !opts->record_origin &&
...adding signoff and recording origin requires not passing commit verbatim,...
> git_config_get_value("commit.cleanup", &value))..., and in other cases are check the "commit.cleanup"...
> argv_array_push(&array, "--cleanup=verbatim");
... and pass commit verbatim if it is not set.
Ah, well, the change you made looks good.
Show 7 quoted lines
> > @@ -781,7 +785,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, > } > if (!opts->no_commit) > res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(), > - opts, allow, opts->edit, 0); > + opts, allow, opts->edit, 0, 0);
The calling convention begins to look unwieldy, but we have only a single such callsite, and there are quite a bit callsites in Git code that have similar API ("git grep ', 0, 0' -- '*.c'"). So we don't need to think about alternatives. Yet.
It's a pity that emulation of named parameters in C requires relying on designated inits from C99
typedef struct {
double pressure, moles, temp;
} ideal_struct; #define ideal_pressure(...) ideal_pressure_base((ideal_struct){.pressure=1, \
.moles=1, .temp=273.15, __VA_ARGS__}) double ideal_pressure_base(ideal_struct in)
{
return 8.314 * in.moles*in.temp/in.pressure;
}... ideal_pressure(.moles=2, .temp=373.15) ...
Show 18 quoted lines
> > leave: > free_message(commit, &msg); > diff --git a/sequencer.h b/sequencer.h > index 2106c0d..e272549 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -50,7 +50,8 @@ int sequencer_rollback(struct replay_opts *opts); > int sequencer_remove_state(struct replay_opts *opts); > > int sequencer_commit(const char *defmsg, struct replay_opts *opts, > - int allow_empty, int edit, int amend); > + int allow_empty, int edit, int amend, > + int cleanup_commit_message); > > extern const char sign_off_header[]; > >