Stefan Sperling <stsp@stsp.name> wrote:
> On Mon, Mar 14, 2022 at 10:18:15PM +0100, Omar Polo wrote:
> > blob - 6e5ec0f2143d39ff9eeca89060522c5155706a77
> > blob + a10f54062e0e80aa2a49665b6f36f51f610d9350
> > --- lib/patch.c
> > +++ lib/patch.c
> > @@ -66,12 +66,19 @@ struct got_patch_hunk {
> >  };
> >  
> >  struct got_patch {
> > +	got_patch_progress_cb cb;
> > +	void	*arg;
> 
> These should no longer be needed? ^
> 
> Oh wait, you did not go all the way.
> patch_add() was not updated to use struct patch_args. More below.
> 
> [...]
> >  const struct got_error *
> >  got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
> > -    int nop, got_worktree_delete_cb delete_cb, void *delete_arg,
> > -    got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb,
> > -    void *cancel_arg)
> > +    int nop, got_patch_progress_cb progress_cb, void *progress_arg,
> > +    got_cancel_cb cancel_cb, void *cancel_arg)
> >  {
> >  	const struct got_error *err = NULL;
> >  	struct imsgbuf *ibuf;
> > @@ -704,9 +730,10 @@ got_patch(int fd, struct got_worktree *worktree, struc
> >  		if (err || done)
> >  			break;
> >  
> > +		p.cb = progress_cb;
> > +		p.arg = progress_arg;
> >  		p.nop = nop;
> 
> Could we pass progress_cb, progress_arg, and nop as separate arguments
> to apply_patch(), instead of stashing them into struct got_patch?

yeah, I forgot half of the fixes...  apologize.  here's an improved
version that also takes nop out of the struct got_patch.  (i initially
put that there to avoid propagating it through some layers of function
call, but it's ugly there and better be explicit)

-----------------------------------------------
commit 75ca5b38a49164eb5fc3837c68b18cbc6c14de3d
from: Omar Polo <op@omarpolo.com>
date: Mon Mar 14 21:43:24 2022 UTC
 
 introduce got_patch_progress_cb
 
 in got_patch I've used callbacks that are intended for other operations.
 This introduce a proper got_patch specific progress callback that is
 invoked after processing a patch.  It also drops the hackish printf in
 the modified case and takes `nop' out of the struct got_patch.
 
diff 463cdc6089041a9ec1747b58910c2f9be8bb6cd7 392f9a1b74053efa310bd05d58851846beb49465
blob - d595de198f4880c3458bb395b994e889acf68df3
blob + 541f9eac1d2cad4c888bcf3da0eb9a7a1632c750
--- got/got.c
+++ got/got.c
@@ -7186,6 +7186,17 @@ patch_from_stdin(int *patchfd)
 }
 
 static const struct got_error *
+patch_progress(void *arg, const char *old, const char *new, unsigned char mode)
+{
+	const char *path = new == NULL ? old : new;
+
+	while (*path == '/')
+		path++;
+	printf("%c  %s\n", mode, path);
+	return NULL;
+}
+
+static const struct got_error *
 cmd_patch(int argc, char *argv[])
 {
 	const struct got_error *error = NULL, *close_error = NULL;
@@ -7247,8 +7258,8 @@ cmd_patch(int argc, char *argv[])
 		err(1, "pledge");
 #endif
 
-	error = got_patch(patchfd, worktree, repo, nop, &print_remove_status,
-	    NULL, &add_progress, NULL, check_cancelled, NULL);
+	error = got_patch(patchfd, worktree, repo, nop, &patch_progress,
+	    NULL, check_cancelled, NULL);
 
 done:
 	if (repo) {
blob - 5f28ffc619f1b4e32ba37ff8e5361c636009ba74
blob + 391c0cf0a69178e759d03ba7c752062f5752dfc6
--- include/got_patch.h
+++ include/got_patch.h
@@ -15,6 +15,14 @@
  */
 
 /*
+ * A callback that gets invoked during the patch application.
+ *
+ * Receives the old and new path and a status code.
+ */
+typedef const struct got_error *(*got_patch_progress_cb)(void *,
+    const char *, const char *, unsigned char);
+
+/*
  * Apply the (already opened) patch to the repository and register the
  * status of the added and removed files.
  *
@@ -22,5 +30,4 @@
  */
 const struct got_error *
 got_patch(int, struct got_worktree *, struct got_repository *, int,
-    got_worktree_delete_cb, void *, got_worktree_checkout_cb, void *,
-    got_cancel_cb, void *);
+    got_patch_progress_cb, void *, got_cancel_cb, void *);
blob - 6e5ec0f2143d39ff9eeca89060522c5155706a77
blob + 5f668442ac2230bb8cd7ade435e4ef6c89434c8c
--- lib/patch.c
+++ lib/patch.c
@@ -66,12 +66,16 @@ struct got_patch_hunk {
 };
 
 struct got_patch {
-	int	 nop;
 	char	*old;
 	char	*new;
 	STAILQ_HEAD(, got_patch_hunk) head;
 };
 
+struct patch_args {
+	got_patch_progress_cb progress_cb;
+	void	*progress_arg;
+};
+
 static const struct got_error *
 send_patch(struct imsgbuf *ibuf, int fd)
 {
@@ -384,7 +388,8 @@ apply_hunk(FILE *tmp, struct got_patch_hunk *h, long *
 }
 
 static const struct got_error *
-patch_file(struct got_patch *p, const char *path, FILE *tmp)
+patch_file(struct got_patch *p, const char *path, FILE *tmp, int nop,
+    struct patch_args *pa)
 {
 	const struct got_error *err = NULL;
 	struct got_patch_hunk *h;
@@ -400,7 +405,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 		h = STAILQ_FIRST(&p->head);
 		if (h == NULL || STAILQ_NEXT(h, entries) != NULL)
 			return got_error(GOT_ERR_PATCH_MALFORMED);
-		if (p->nop)
+		if (nop)
 			return NULL;
 		for (i = 0; i < h->len; ++i) {
 			if (fprintf(tmp, "%s", h->lines[i]+1) < 0)
@@ -423,7 +428,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 		err = locate_hunk(orig, h, &pos, &lineno);
 		if (err != NULL)
 			goto done;
-		if (!p->nop)
+		if (!nop)
 			err = copy(tmp, orig, copypos, pos);
 		if (err != NULL)
 			goto done;
@@ -450,7 +455,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 		if (err != NULL)
 			goto done;
 
-		if (!p->nop)
+		if (!nop)
 			err = apply_hunk(tmp, h, &lineno);
 		if (err != NULL)
 			goto done;
@@ -470,7 +475,7 @@ patch_file(struct got_patch *p, const char *path, FILE
 			err = got_error_from_errno("fstat");
 		else if (sb.st_size != copypos)
 			err = got_error(GOT_ERR_PATCH_DONT_APPLY);
-	} else if (!p->nop && !feof(orig))
+	} else if (!nop && !feof(orig))
 		err = copy(tmp, orig, copypos, -1);
 
 done:
@@ -565,10 +570,26 @@ check_file_status(struct got_patch *p, int file_rename
 }
 
 static const struct got_error *
+patch_delete(void *arg, unsigned char status, unsigned char staged_status,
+    const char *path)
+{
+	struct patch_args *pa = arg;
+
+	return pa->progress_cb(pa->progress_arg, path, NULL, status);
+}
+
+static const struct got_error *
+patch_add(void *arg, unsigned char status, const char *path)
+{
+	struct patch_args *pa = arg;
+
+	return pa->progress_cb(pa->progress_arg, NULL, path, status);
+}
+
+static const struct got_error *
 apply_patch(struct got_worktree *worktree, struct got_repository *repo,
-    struct got_patch *p, got_worktree_delete_cb delete_cb, void *delete_arg,
-    got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb,
-    void *cancel_arg)
+    struct got_patch *p, int nop, struct patch_args *pa,
+    got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
 	struct got_pathlist_head oldpaths, newpaths;
@@ -604,20 +625,20 @@ apply_patch(struct got_worktree *worktree, struct got_
 		goto done;
 	}
 
-	if (!p->nop)
+	if (!nop)
 		err = got_opentemp_named(&tmppath, &tmp, template);
 	if (err)
 		goto done;
-	err = patch_file(p, oldpath, tmp);
+	err = patch_file(p, oldpath, tmp, nop, pa);
 	if (err)
 		goto done;
 
-	if (p->nop)
+	if (nop)
 		goto done;
 
 	if (p->old != NULL && p->new == NULL) {
 		err = got_worktree_schedule_delete(worktree, &oldpaths,
-		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
+		    0, NULL, patch_delete, pa, repo, 0, 0);
 		goto done;
 	}
 
@@ -628,15 +649,16 @@ apply_patch(struct got_worktree *worktree, struct got_
 
 	if (file_renamed) {
 		err = got_worktree_schedule_delete(worktree, &oldpaths,
-		    0, NULL, delete_cb, delete_arg, repo, 0, 0);
+		    0, NULL, patch_delete, pa, repo, 0, 0);
 		if (err == NULL)
 			err = got_worktree_schedule_add(worktree, &newpaths,
-			    add_cb, add_arg, repo, 1);
+			    patch_add, pa, repo, 1);
 	} else if (p->old == NULL)
 		err = got_worktree_schedule_add(worktree, &newpaths,
-		    add_cb, add_arg, repo, 1);
+		    patch_add, pa, repo, 1);
 	else
-		printf("M  %s\n", oldpath); /* XXX */
+		err = pa->progress_cb(pa->progress_arg, oldpath, newpath,
+		    GOT_STATUS_MODIFY);
 
 done:
 	if (err != NULL && newpath != NULL && (file_renamed || p->old == NULL))
@@ -654,16 +676,19 @@ done:
 
 const struct got_error *
 got_patch(int fd, struct got_worktree *worktree, struct got_repository *repo,
-    int nop, got_worktree_delete_cb delete_cb, void *delete_arg,
-    got_worktree_checkout_cb add_cb, void *add_arg, got_cancel_cb cancel_cb,
-    void *cancel_arg)
+    int nop, got_patch_progress_cb progress_cb, void *progress_arg,
+    got_cancel_cb cancel_cb, void *cancel_arg)
 {
 	const struct got_error *err = NULL;
+	struct patch_args pa;
 	struct imsgbuf *ibuf;
 	int imsg_fds[2] = {-1, -1};
 	int done = 0;
 	pid_t pid;
 
+	pa.progress_cb = progress_cb;
+	pa.progress_arg = progress_arg;
+
 	ibuf = calloc(1, sizeof(*ibuf));
 	if (ibuf == NULL) {
 		err = got_error_from_errno("calloc");
@@ -704,9 +729,8 @@ got_patch(int fd, struct got_worktree *worktree, struc
 		if (err || done)
 			break;
 
-		p.nop = nop;
-		err = apply_patch(worktree, repo, &p, delete_cb, delete_arg,
-		    add_cb, add_arg, cancel_cb, cancel_arg);
+		err = apply_patch(worktree, repo, &p, nop, &pa,
+		    cancel_cb, cancel_arg);
 		patch_free(&p);
 		if (err)
 			break;