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.

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

git-add--interactive.perl: Add progress counter in the prompt #349

Closed
wants to merge 1 commit into from
Closed

Conversation

kunaltyagi
Copy link

@kunaltyagi kunaltyagi commented Sep 21, 2019

Hi git contributors!

I'm Kunal Tyagi. While I was choosing the relevant patches for a commit using the git add -p command, I found that there was no feedback regarding how many hunks from the current file had been processed and how many were left. So I decided to add a small change to the prompt which basically just displays (${current-hunk-id} + 1/${total-hunks}) before displaying the prompt during user interaction. This patch doesn't account for all total hunks, only per file.

I don't know perl so even this one liner might have mistakes. I did test this locally and hope this works for others. Personally, this change feels helpful to me when I have to separate a long list of changes after an erroneous commit.

On the #git-devel freenode channel, I was informed that @dscho is rewriting git-add in C. If so, perhaps a similar change could be added in the rewrite. I haven't seen his patches in detail so I can't comment if it'll be as trivial as in perl.

Regards
Kunal Tyagi

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 21, 2019

Welcome to GitGitGadget

Hi @kunaltyagi, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests.

Please make sure that this Pull Request has a good description, as it will be used as cover letter.

Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:

  • the lines should not exceed 76 columns,
  • the first line should be like a header and typically start with a prefix like "tests:" or "commit:", and
  • the commit messages' body should be describing the "why?" of the change.
  • Finally, the commit messages should end in a Signed-off-by: line matching the commits' author.

It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code.

Contributing the patches

Before you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a PR comment of the form /allow <username>.

Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment /submit.

After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions.

If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox ("raw") file corresponding to the mail you want to reply to from the Git mailing list. If you use GMail, you can upload that raw mbox file via:

curl -g --user "<EMailAddress>:<Password>" --url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txt

@dscho
Copy link
Member

dscho commented Sep 22, 2019

/allow

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 22, 2019

User kunaltyagi is now allowed to use GitGitGadget.

@kunaltyagi
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 23, 2019

Submitted as pull.349.git.gitgitgadget@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 25, 2019

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi,

On Mon, 23 Sep 2019, Kunal Tyagi via GitGitGadget wrote:

> Hi git contributors!
>
> I'm Kunal Tyagi. While I was choosing the relevant patches for a commit
> using the git add -p command, I found that there was no feedback regardi=
ng
> how many hunks from the current file had been processed and how many wer=
e
> left. So I decided to add a small change to the prompt which basically j=
ust
> displays (${current-hunk-id} + 1/${total-hunks}) before displaying the
> prompt during user interaction. This patch doesn't account for all total
> hunks, only per file.

I like this idea!

> I don't know perl so even this one liner might have mistakes. I did test
> this locally and hope this works for others. Personally, this change fee=
ls
> helpful to me when I have to separate a long list of changes after an
> erroneous commit.
>
> On the #git-devel freenode channel, I was informed that @dscho is rewrit=
ing
> git-add in C. If so, perhaps a similar change could be added in the rewr=
ite.
> I haven't seen his patches in detail so I can't comment if it'll be as
> trivial as in perl.

I bet it will be trivial to implement it in C. Don't let this stop you,
your change should go in before built-in add -i.

Thanks,
Johannes

>
> Regards Kunal Tyagi
>
> Kunal Tyagi (1):
>   git-add--interactive.perl: Add progress counter in the prompt
>
>  git-add--interactive.perl  | 2 +-
>  t/t3701-add-interactive.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
>
> base-commit: 4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-349%2F=
kunaltyagi%2Fmaster-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-349/kunal=
tyagi/master-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/349
> --
> gitgitgadget
>

@kunaltyagi
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 29, 2019

Submitted as pull.349.v2.git.gitgitgadget@gmail.com

Report the current hunk count and total number of hunks for the current
file in the prompt
Adjust the expected output in some tests to account for new data on the prompt

Signed-off-by: Kunal Tyagi <tyagi.kunal@live.com>
@kunaltyagi
Copy link
Author

kunaltyagi commented Sep 30, 2019

Hi

Thanks for your comments. I've modified the patch and I hope it is more suitable now.

I just used a repl to get the output and sent that as a patch. I don't really know perl (though printf syntax is almost universal).

Cheers
Kunal Tyagi

@kunaltyagi
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Sep 30, 2019

Submitted as pull.349.v3.git.gitgitgadget@gmail.com

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This branch is now known as kt/add-i-progress.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

This patch series was integrated into pu via git@60d083e.

@gitgitgadget gitgitgadget bot added the pu label Oct 4, 2019
@@ -1541,7 +1541,7 @@ sub patch_update_file {
for (@{$hunk[$ix]{DISPLAY}}) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Kunal Tyagi via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Kunal Tyagi <tyagi.kunal@live.com>
>
> Report the current hunk count and total number of hunks for the current
> file in the prompt
> Adjust the expected output in some tests to account for new data on the prompt
>
> Signed-off-by: Kunal Tyagi <tyagi.kunal@live.com>
> ---
>  git-add--interactive.perl  | 2 +-
>  t/t3701-add-interactive.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Thanks; queued.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 4, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 6, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 7, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 7, 2019

This patch series was integrated into next via git@00cf8fe.

@gitgitgadget gitgitgadget bot added the next label Oct 7, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 8, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 9, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

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

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

This patch series was integrated into next via git@f0d407e.

@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

This patch series was integrated into master via git@f0d407e.

@gitgitgadget gitgitgadget bot added the master label Oct 11, 2019
@gitgitgadget
Copy link

gitgitgadget bot commented Oct 11, 2019

Closed via f0d407e.

@gitgitgadget gitgitgadget bot closed this Oct 11, 2019
@dscho
Copy link
Member

dscho commented Oct 11, 2019

🎉

@kunaltyagi
Copy link
Author

I was actually wondering if it would be an improvement to add information regarding total files or hunks being considered instead of per file (possibly in the C version not the perl one)

Progress: 4/9 hunks this file; 30 hunks in 2 files
Add this hunk? (y/n....): 

@dscho
Copy link
Member

dscho commented Oct 11, 2019

It would be an improvement, but the current code is really focused on individual files... Therefore it would take quite a bit more surgery to get that information.

@kunaltyagi
Copy link
Author

quite a bit more surgery

Which branch is the C development going on?

@dscho
Copy link
Member

dscho commented Oct 12, 2019

Which branch is the C development going on?

It's developed (slowly) in #170, #171, #172, #173, #174, and #175.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants