git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH v2 0/9] built-in add -i: implement all commands in the main loop
Date: Fri, 29 Nov 2019 21:11:40 +0000
Message-ID: <pull.171.v2.git.1575061909.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.171.git.1573821382.gitgitgadget@gmail.com>

Based on the js/builtin-add-i branch, this patch series implements the rest
of the commands in git add -i's main loop: update, revert, add_untracked, 
patch, diff, and quit. Apart from quit, these commands are all very similar
in that they first build a list of files, display it, let the user choose
which ones to act on, and then perform the action.

Note that the patch command is not actually converted to C, not completely
at least: the built-in version simply hands off to git add--interactive 
after letting the user select which files to act on.

The reason for this omission is practicality. Out of the 1,800+ lines of 
git-add--interactive.perl, over a thousand deal just with the git add -p 
part. I did convert that functionality already (to be contributed in a
separate patch series, see https://github.com/gitgitgadget/git/pull/173),
discovering that there is so little overlap between the git add --patch part
and the rest of git add --interactive that I could put the former into a
totally different file: add-patch.c.

Changes since v1:

 * As an introductory commit, we now release the pathspecs that we passed to
   the diff machinery.
 * The ternary in 1/8 (now 1/9) was adjusted from i ? to (i == 0) ? to make
   the flow more intuitive.
 * The commit message of 2/8 (now 3/9) no longer talks about an upgrade 
   command (which never existed)but about the update command (that I
   actually meant).

Johannes Schindelin (9):
  add-interactive: make sure to release `rev.prune_data`
  built-in add -i: allow filtering the modified files list
  built-in add -i: prepare for multi-selection commands
  built-in add -i: implement the `update` command
  built-in add -i: re-implement `revert` in C
  built-in add -i: re-implement `add-untracked` in C
  built-in add -i: implement the `patch` command
  built-in add -i: re-implement the `diff` command
  built-in add -i: offer the `quit` command

 add-interactive.c | 619 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 560 insertions(+), 59 deletions(-)


base-commit: 8c159044625e46de67cd8467f07424f38eb8301e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-171%2Fdscho%2Fadd-i-in-c-all-except-patch-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-171/dscho/add-i-in-c-all-except-patch-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/171

Range-diff vs v1:

  -:  ---------- >  1:  1e02bf2d63 add-interactive: make sure to release `rev.prune_data`
  1:  1844c4d55e !  2:  fab098d86e built-in add -i: allow filtering the modified files list
     @@ -17,12 +17,21 @@
          and `diff-files` phases, and then skipping files in the second phase
          when they have not been seen in the first phase.
      
     +    Seeing as we change the meaning of the `phase` field, we rename it to
     +    `mode` to reflect that the order depends on the exact invocation of the
     +    `git add -i` command.
     +
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       diff --git a/add-interactive.c b/add-interactive.c
       --- a/add-interactive.c
       +++ b/add-interactive.c
      @@
     + }
     + 
     + struct collection_status {
     +-	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } phase;
     ++	enum { FROM_WORKTREE = 0, FROM_INDEX = 1 } mode;
       
       	const char *reference;
       
     @@ -40,6 +49,15 @@
       			add_file_item(s->files, name);
       
       			entry = xcalloc(sizeof(*entry), 1);
     +@@
     + 		}
     + 
     + 		file_item = entry->item;
     +-		adddel = s->phase == FROM_INDEX ?
     ++		adddel = s->mode == FROM_INDEX ?
     + 			&file_item->index : &file_item->worktree;
     + 		adddel->seen = 1;
     + 		adddel->add = stat.files[i]->added;
      @@
       	free_diffstat_info(&stat);
       }
     @@ -59,7 +77,8 @@
       	struct object_id head_oid;
       	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
       					     &head_oid, NULL);
     - 	struct collection_status s = { FROM_WORKTREE };
     +-	struct collection_status s = { FROM_WORKTREE };
     ++	struct collection_status s = { 0 };
      +	int i;
       
       	if (discard_index(r->index) < 0 ||
     @@ -74,14 +93,23 @@
       		struct setup_revision_opt opt = { 0 };
       
      +		if (filter == INDEX_ONLY)
     -+			s.phase = i ? FROM_WORKTREE : FROM_INDEX;
     ++			s.mode = (i == 0) ? FROM_INDEX : FROM_WORKTREE;
      +		else
     -+			s.phase = i ? FROM_INDEX : FROM_WORKTREE;
     ++			s.mode = (i == 0) ? FROM_WORKTREE : FROM_INDEX;
      +		s.skip_unseen = filter && i;
      +
       		opt.def = is_initial ?
       			empty_tree_oid_hex() : oid_to_hex(&head_oid);
       
     +@@
     + 		if (ps)
     + 			copy_pathspec(&rev.prune_data, ps);
     + 
     +-		if (s.phase == FROM_INDEX)
     ++		if (s.mode == FROM_INDEX)
     + 			run_diff_index(&rev, 1);
     + 		else {
     + 			rev.diffopt.flags.ignore_dirty_submodules = 1;
      @@
       static int run_status(struct add_i_state *s, const struct pathspec *ps,
       		      struct string_list *files, struct list_options *opts)
  2:  c324b47592 !  3:  58a581f4ee built-in add -i: prepare for multi-selection commands
     @@ -2,7 +2,7 @@
      
          built-in add -i: prepare for multi-selection commands
      
     -    The `upgrade`, `revert` and `add-untracked` commands allow selecting
     +    The `update`, `revert` and `add-untracked` commands allow selecting
          multiple entries. Let's extend the `list_and_choose()` function to
          accommodate those use cases.
      
     @@ -168,7 +168,8 @@
      +				}
       			}
       
     - 			p[sep] = '\0';
     + 			if (p[sep])
     + 				p[sep++] = '\0';
      -			if (index < 0)
      -				index = find_unique(p, items);
      +			if (from < 0) {
     @@ -199,7 +200,7 @@
      +					res += choose ? +1 : -1;
      +				}
      +
     - 			p += sep + 1;
     + 			p += sep;
       		}
       
      -		if (res != LIST_AND_CHOOSE_ERROR)
  3:  cba5f13152 =  4:  c311a29c77 built-in add -i: implement the `update` command
  4:  5c31bbd24a =  5:  f70723a160 built-in add -i: re-implement `revert` in C
  5:  5c498795b3 =  6:  952fbc8f79 built-in add -i: re-implement `add-untracked` in C
  6:  0c8a71e2e8 !  7:  cbd10da523 built-in add -i: implement the `patch` command
     @@ -51,9 +51,9 @@
       						    struct pathname_entry, ent);
      @@
       		file_item = entry->item;
     - 		adddel = s->phase == FROM_INDEX ?
     + 		adddel = s->mode == FROM_INDEX ?
       			&file_item->index : &file_item->worktree;
     -+		other_adddel = s->phase == FROM_INDEX ?
     ++		other_adddel = s->mode == FROM_INDEX ?
      +			&file_item->worktree : &file_item->index;
       		adddel->seen = 1;
       		adddel->add = stat.files[i]->added;
     @@ -84,7 +84,7 @@
       	struct object_id head_oid;
       	int is_initial = !resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
      @@
     - 		}
     + 			clear_pathspec(&rev.prune_data);
       	}
       	hashmap_free_entries(&s.file_map, struct pathname_entry, ent);
      +	if (unmerged_count)
  7:  499f3f19e0 =  8:  e4907112e4 built-in add -i: re-implement the `diff` command
  8:  dbcb340ede =  9:  5ba6cd31ab built-in add -i: offer the `quit` command

-- 
gitgitgadget

  parent reply index

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-15 12:36 [PATCH 0/8] build-in " Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 1/8] built-in add -i: allow filtering the modified files list Johannes Schindelin via GitGitGadget
2019-11-21  8:06   ` Junio C Hamano
2019-11-25 15:20     ` Johannes Schindelin
2019-11-15 12:36 ` [PATCH 2/8] built-in add -i: prepare for multi-selection commands Johannes Schindelin via GitGitGadget
2019-11-21  8:12   ` Junio C Hamano
2019-11-25 15:21     ` Johannes Schindelin
2019-11-15 12:36 ` [PATCH 3/8] built-in add -i: implement the `update` command Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 4/8] built-in add -i: re-implement `revert` in C Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 5/8] built-in add -i: re-implement `add-untracked` " Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 6/8] built-in add -i: implement the `patch` command Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 7/8] built-in add -i: re-implement the `diff` command Johannes Schindelin via GitGitGadget
2019-11-15 12:36 ` [PATCH 8/8] built-in add -i: offer the `quit` command Johannes Schindelin via GitGitGadget
2019-11-18  2:17 ` [PATCH 0/8] build-in add -i: implement all commands in the main loop Junio C Hamano
2019-11-18  2:22   ` Junio C Hamano
2019-11-18 19:22     ` Johannes Schindelin
2019-11-18  2:27 ` Junio C Hamano
2019-11-18 18:53   ` Johannes Schindelin
2019-11-19  1:29     ` Junio C Hamano
2019-11-29 21:11 ` Johannes Schindelin via GitGitGadget [this message]
2019-11-29 21:11   ` [PATCH v2 1/9] add-interactive: make sure to release `rev.prune_data` Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 2/9] built-in add -i: allow filtering the modified files list Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 3/9] built-in add -i: prepare for multi-selection commands Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 4/9] built-in add -i: implement the `update` command Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 5/9] built-in add -i: re-implement `revert` in C Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 6/9] built-in add -i: re-implement `add-untracked` " Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 7/9] built-in add -i: implement the `patch` command Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 8/9] built-in add -i: re-implement the `diff` command Johannes Schindelin via GitGitGadget
2019-11-29 21:11   ` [PATCH v2 9/9] built-in add -i: offer the `quit` command Johannes Schindelin via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.171.v2.git.1575061909.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git