Carlo Arenas <care...@gmail.com> writes:

> would something like this work better? (not to apply, and probably mangled)

At least call it "create_empty_file(path)" instead.

"touch" is primarily to update the last-modified-time timestamp of a
file.  If the file does not exist, it is created while doing so, but
when readers of your code sees a "touch", you make them anticipate
that you are relying on file timestamp somehow.  Using it to create
an empty file wastes time of readers who read your code by forcing
them wonder why you care about the file timestamp.

For a single-use, not using the macro and just using "%s", "" should
suffice.  If we want to hide the "%s", "" trickery from distracting
the readers (which is what you are trying to address with your
touch_file() proposal, I think, and I also suspect that it may be a
legitimate one), I tend to think that it may be a better solution to
introduce the EMPTY_PRINTF_ARG macro and use it everywhere, in
builtin/commit.c, builtin/difftool.c and wt-status.c and not not
just here.

> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -35,6 +35,8 @@
>
>  #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> +#define touch_file(path) write_file(path, "%s", "")
> +
>  const char sign_off_header[] = "Signed-off-by: ";
>  static const char cherry_picked_prefix[] = "(cherry picked from commit ";
>
> @@ -2389,7 +2391,7 @@ int write_basic_state(struct replay_opts *opts,
> const char *head_name,
>                 write_file(rebase_path_quiet(), "\n");
>
>         if (opts->verbose)
> -               write_file(rebase_path_verbose(), "");
> +               touch_file(rebase_path_verbose());
>         if (opts->strategy)
>                 write_file(rebase_path_strategy(), "%s\n", opts->strategy);
>         if (opts->xopts_nr > 0)

Reply via email to