Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

built-in add -i: implement all commands in the main loop #171

Closed
wants to merge 9 commits into from
Closed

built-in add -i: implement all commands in the main loop #171

wants to merge 9 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Mar 28, 2019 •
edited
Loading

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 #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).

@dscho dscho force-pushed the add-i-in-c-all-except-patch branch 3 times, most recently from eabb1c1 to bf63aea Compare April 3, 2019 09:40
This was referenced Apr 10, 2019
Merged
Closed
@dscho dscho force-pushed the add-i-in-c-all-except-patch branch from bf63aea to 32532b1 Compare April 10, 2019 17:42
@dscho dscho mentioned this pull request Sep 24, 2019
Closed
@dscho dscho mentioned this pull request Oct 12, 2019
Closed
@dscho dscho force-pushed the add-i-in-c-all-except-patch branch from 32532b1 to d7764a8 Compare November 14, 2019 09:36
@dscho dscho changed the base branch from master to js/builtin-add-i November 14, 2019 09:45
@dscho dscho force-pushed the add-i-in-c-all-except-patch branch from d7764a8 to dbcb340 Compare November 14, 2019 14:30
@dscho
Copy link
Member Author

dscho commented Nov 15, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 15, 2019

Submitted as pull.171.git.1573821382.gitgitgadget@gmail.com

@gitgitgadget

This comment has been minimized.

Sign in to view
@gitgitgadget

This comment has been minimized.

Sign in to view
@gitgitgadget

This comment has been minimized.

Sign in to view
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 18, 2019

This branch is now known as js/builtin-add-i-cmds.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 18, 2019

This patch series was integrated into pu via git@e5bdcde.

@gitgitgadget gitgitgadget bot added the pu label Nov 18, 2019
@gitgitgadget

This comment has been minimized.

Sign in to view
@gitgitgadget

This comment has been minimized.

Sign in to view
@dscho dscho force-pushed the add-i-in-c-all-except-patch branch from dbcb340 to c46d104 Compare November 18, 2019 22:00
@gitgitgadget

This comment has been minimized.

Sign in to view
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2019

This patch series was integrated into pu via git@35ece5a.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 19, 2019

This patch series was integrated into pu via git@5caf9d1.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 20, 2019

This patch series was integrated into pu via git@635b7ae.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 20, 2019

This patch series was integrated into pu via git@06fa998.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 21, 2019

This patch series was integrated into pu via git@dd35268.

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 21, 2019

This patch series was integrated into pu via git@f087fee.

@dscho
add-interactive: make sure to release rev.prune_data
1e02bf2 
During a review, Junio Hamano pointed out that the `rev.prune_data` was
copied from another pathspec but never cleaned up.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: allow filtering the modified files list
fab098d 
In the `update` command of `git add -i`, we are primarily interested in the
list of modified files that have worktree (i.e. unstaged) changes.

At the same time, we need to determine _also_ the staged changes, to be
able to produce the full added/deleted information.

The Perl script version of `git add -i` has a parameter of the
`list_modified()` function for that matter. In C, we can be a lot more
precise, using an `enum`.

The C implementation of the filter also has an easier time to avoid
unnecessary work, simply by using an adaptive order of the `diff-index`
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>
@dscho
built-in add -i: prepare for multi-selection commands
58a581f 
The `update`, `revert` and `add-untracked` commands allow selecting
multiple entries. Let's extend the `list_and_choose()` function to
accommodate those use cases.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: implement the update command
c311a29 
After `status` and `help`, it is now time to port the `update` command
to C, the second command that is shown in the main loop menu of `git add
-i`.

This `git add -i` command is the first one which lets the user choose a
subset of a list of files, and as such, this patch lays the groundwork
for the other commands of that category:

- It teaches the `print_file_item()` function to show a unique prefix
  if we found any (the code to find it had been added already in the
  previous patch where we colored the unique prefixes of the main loop
  commands, but that patch uses the `print_command_item()` function to
  display the menu items).

- This patch also adds the help text that is shown when the user input
  to select items from the shown list could not be parsed.

- As `get_modified_files()` clears the list of files, it now has to take
  care of clearing the _full_ `prefix_item_list` lest the `sorted` and
  `selected` fields go stale and inconsistent.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: re-implement revert in C
f70723a 
This is a relatively straight-forward port from the Perl version, with
the notable exception that we imitate `git reset -- <paths>` in the C
version rather than the convoluted `git ls-tree HEAD -- <paths> | git
update-index --index-info` followed by `git update-index --force-remove
-- <paths>` for the missed ones.

While at it, we fix the pretty obvious bug where the `revert` command
offers to unstage files that do not have staged changes.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: re-implement add-untracked in C
952fbc8 
This is yet another command, ported to C. It builds nicely on the
support functions introduced for other commands, with the notable
difference that only names are displayed for untracked files, no
file type or diff summary.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: implement the patch command
cbd10da 
Well, it is not a full implementation yet. In the interest of making
this easy to review (and easy to keep bugs out), we still hand off to
the Perl script to do the actual work.

The `patch` functionality actually makes up for more than half of the
1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl
to C incrementally, later.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: re-implement the diff command
e490711 
It is not only laziness that we simply spawn `git diff -p --cached`
here: this command needs to use the pager, and the pager needs to exit
when the diff is done. Currently we do not have any way to make that
happen if we run the diff in-process. So let's just spawn.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
built-in add -i: offer the quit command
5ba6cd3 
We do not really want to `exit()` here, of course, as this is safely
libified code.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho force-pushed the add-i-in-c-all-except-patch branch from 8015969 to 5ba6cd3 Compare November 25, 2019 15:34
@gitgitgadget
Copy link

gitgitgadget bot commented Nov 27, 2019

This patch series was integrated into pu via git@eed922c.

@dscho
Copy link
Member Author

dscho commented Nov 29, 2019

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Nov 29, 2019

Submitted as pull.171.v2.git.1575061909.gitgitgadget@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 2, 2019

This patch series was integrated into pu via git@ad3afae.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 3, 2019

This patch series was integrated into pu via git@79d123e.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 5, 2019

This patch series was integrated into pu via git@1f2be32.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 5, 2019

This patch series was integrated into next via git@30cfe5c.

@gitgitgadget gitgitgadget bot added the next label Dec 5, 2019
dscho added a commit to dscho/git that referenced this pull request Dec 6, 2019
@dscho
fixup! built-in add -i: implement the patch command
5b75b86 
This change was introduced in v2 of the PR at
gitgitgadget#171.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho mentioned this pull request Dec 6, 2019
Merged
@gitgitgadget
Copy link

gitgitgadget bot commented Dec 10, 2019

This patch series was integrated into pu via git@bae3920.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 10, 2019

This patch series was integrated into pu via git@013bc80.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 10, 2019

This patch series was integrated into pu via git@379e7f1.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 16, 2019

This patch series was integrated into pu via git@3beff38.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 16, 2019

This patch series was integrated into next via git@3beff38.

@gitgitgadget
Copy link

gitgitgadget bot commented Dec 16, 2019

This patch series was integrated into master via git@3beff38.

@gitgitgadget gitgitgadget bot added the master label Dec 16, 2019
@gitgitgadget gitgitgadget bot closed this Dec 16, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Dec 16, 2019

Closed via 3beff38.

@dscho dscho deleted the add-i-in-c-all-except-patch branch December 17, 2019 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
master next pu
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@dscho