diff mbox series

[03/20] editor: do not rely on `the_repository` for interactive edits

Message ID c2556fff9e25aabab237c5902aff64f65b0a953f.1723013714.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series Stop using `the_repository` in "config.c" | expand

Commit Message

Patrick Steinhardt Aug. 7, 2024, 6:56 a.m. UTC
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
dependency.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 add-patch.c |  3 ++-
 editor.c    | 12 +++++++-----
 editor.h    |  3 ++-
 3 files changed, 11 insertions(+), 7 deletions(-)

Comments

Justin Tobler Aug. 9, 2024, 5:36 p.m. UTC | #1
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@pks.im>
> ---
>  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
>
diff mbox series

Patch

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;
+	}
 
 	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