Re: [PATCH 19/22] sequencer: support cleaning up commit messages

Hello Johannes,

W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:

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 <redacted>
---
 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.

All right, though it slowly begins coming close to the threshold
where using bitfield flags would be sensible.

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.

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) ...