On 24/08/07 08:56AM, Patrick Steinhardt wrote:
> We implicitly rely on `the_repository` when editing a file interactively
> because we call `git_path()`. Adapt the function to instead take a
> `sturct repository` as parameter so that we can remove this hidden
s/sturct/struct/
s/parameter/a parameter/
> dependency.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> add-patch.c | 3 ++-
> editor.c | 12 +++++++-----
> editor.h | 3 ++-
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/add-patch.c b/add-patch.c
> index 46f6bddfe5..4e3aa02ed8 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1140,7 +1140,8 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
> "removed, then the edit is\n"
> "aborted and the hunk is left unchanged.\n"));
>
> - if (strbuf_edit_interactively(&s->buf, "addp-hunk-edit.diff", NULL) < 0)
> + if (strbuf_edit_interactively(&s->buf, "addp-hunk-edit.diff", NULL,
> + the_repository) < 0)
> return -1;
>
> /* strip out commented lines */
> diff --git a/editor.c b/editor.c
> index d1ba2d7c34..6c461dd253 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -134,13 +134,15 @@ int launch_sequence_editor(const char *path, struct strbuf *buffer,
> }
>
> int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
> - const char *const *env)
> + const char *const *env, struct repository *r)
> {
> - char *path2 = NULL;
> + struct strbuf sb = STRBUF_INIT;
> int fd, res = 0;
>
> - if (!is_absolute_path(path))
> - path = path2 = xstrdup(git_path("%s", path));
> + if (!is_absolute_path(path)) {
> + strbuf_repo_git_path(&sb, r, "%s", path);
> + path = sb.buf;
> + }
By switching from `git_path()` to `strbuf_repo_git_path()`,
`strbuf_edit_interactively()` no longer implicitly depends on
`the_repository`. This requires a `struct repository` be injected into
`strbuf_edit_interactively()`. The function signature is updated along
with the single call site. The small cleanup also looks good. Makes
sense to me.
Non-blocking: Instead of appending `struct repository` as the last
argument, it might be nicer to match `strbuf_repo_git_path()` and place
it as the second argument.
> fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0666);
> if (fd < 0)
> @@ -157,6 +159,6 @@ int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
> unlink(path);
> }
>
> - free(path2);
> + strbuf_release(&sb);
> return res;
> }
> diff --git a/editor.h b/editor.h
> index 8016bb5e00..d4c4d216ac 100644
> --- a/editor.h
> +++ b/editor.h
> @@ -1,6 +1,7 @@
> #ifndef EDITOR_H
> #define EDITOR_H
>
> +struct repository;
> struct strbuf;
>
> const char *git_editor(void);
> @@ -29,6 +30,6 @@ int launch_sequence_editor(const char *path, struct strbuf *buffer,
> * If `path` is relative, it refers to a file in the `.git` directory.
> */
> int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
> - const char *const *env);
> + const char *const *env, struct repository *r);
>
> #endif
> --
> 2.46.0.dirty
>
