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

Add prompt-yes config setting for help.autocorrect #1852

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asilano
Copy link

@asilano asilano commented Jan 14, 2025

This is my first patch request to git - please do let me know if I should be doing something differently!

cc: "Kristoffer Haugsbakk" [email protected]

Copy link

gitgitgadget bot commented Jan 14, 2025

Welcome to GitGitGadget

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

Please make sure that either:

  • Your Pull Request has a good description, if it consists of multiple commits, as it will be used as cover letter.
  • Your Pull Request description is empty, if it consists of a single commit, as the commit message should be descriptive enough by itself.

You can CC potential reviewers by adding a footer to the PR description with the following syntax:

CC: Revi Ewer <[email protected]>, Ill Takalook <[email protected]>

NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description,
because it will result in a malformed CC list on the mailing list. See
example.

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 "revisions:" to state which subsystem the change is about, 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 comment to your PR of the form /allow. A good way to find other contributors is to locate recent pull requests where someone has been /allowed:

Both the person who commented /allow and the PR author are able to /allow you.

An alternative is the channel #git-devel on the Libera Chat IRC network:

<newcontributor> I've just created my first PR, could someone please /allow me? https://github.com/gitgitgadget/git/pull/12345
<veteran> newcontributor: it is done
<newcontributor> thanks!

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

If you want to see what email(s) would be sent for a /submit request, add a PR comment /preview to have the email(s) sent to you. You must have a public GitHub email address for this. Note that any reviewers CC'd via the list in the PR description will not actually be sent emails.

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 (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail).

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 from the Git mailing list archive (click the (raw) link), then import it into your mail program. If you use GMail, you can do this via:

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

To iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description):

Changes since v1:
- Fixed a typo in the commit message (found by ...)
- Added a code comment to ... as suggested by ...
...

To send a new iteration, just add another PR comment with the contents: /submit.

Need help?

New contributors who want advice are encouraged to join [email protected], where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join.

You may also be able to find help in real time in the developer IRC channel, #git-devel on Libera Chat. Remember that IRC does not support offline messaging, so if you send someone a private message and log out, they cannot respond to you. The scrollback of #git-devel is archived, though.

Copy link

gitgitgadget bot commented Jan 14, 2025

There are issues in commit 2c21ad8:
Add prompt-yes config setting for help.autocorrect
Commit not signed off

@asilano asilano force-pushed the autocorrect-allow-prompt-default-yes branch from 2c21ad8 to 4f393f9 Compare January 15, 2025 09:16
Copy link

gitgitgadget bot commented Jan 15, 2025

There are issues in commit 4f393f9:
help: Add prompt-yes setting for autocorrect
Prefixed commit message must be in lower case
Commit not signed off

@asilano asilano force-pushed the autocorrect-allow-prompt-default-yes branch from 4f393f9 to cc6824c Compare January 15, 2025 09:17
Copy link

gitgitgadget bot commented Jan 15, 2025

There are issues in commit cc6824c:
help: Add prompt-yes setting for autocorrect
Prefixed commit message must be in lower case

@asilano asilano force-pushed the autocorrect-allow-prompt-default-yes branch from cc6824c to 8827e6b Compare January 15, 2025 09:19
The help.autocorrect functionality is really useful, saving frustration
when a dev fat-fingers a command, and git has a pretty good idea what
was originally intended. The config settings are a nice selection, with
"prompt" asking the user to confirm that they want to run the assumed
command.

However, with "prompt", the choice defaults to "No" - that is, hitting
return will _not_ run the command. For me at least, if git is confident
it knows which command I wanted, it's usually right, and the golden path
would be to run the command.

Therefore this patch adds "prompt-yes" as a counterpart config setting
for help.autocorrect, which does the same as "prompt", but defaults to
"Yes" - hitting return will run the assumed command.

I have not added any tests because the test suite doesn't have any tests
(that I could find) for the "prompt" behaviour - I'm assuming this is
because it's hard/impossible to simulate the interactive terminal prompt

Signed-off-by: Chris Howlett <[email protected]>
@asilano asilano force-pushed the autocorrect-allow-prompt-default-yes branch from 8827e6b to b87c772 Compare January 15, 2025 09:21
@pks-t
Copy link

pks-t commented Jan 15, 2025

/allow

Copy link

gitgitgadget bot commented Jan 15, 2025

User asilano is now allowed to use GitGitGadget.

WARNING: asilano has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use.

@pks-t
Copy link

pks-t commented Jan 15, 2025

/allow

Copy link

gitgitgadget bot commented Jan 15, 2025

User asilano already allowed to use GitGitGadget.

@asilano
Copy link
Author

asilano commented Jan 15, 2025

/preview

Copy link

gitgitgadget bot commented Jan 15, 2025

Preview email sent as [email protected]

@asilano
Copy link
Author

asilano commented Jan 15, 2025

/submit

Copy link

gitgitgadget bot commented Jan 15, 2025

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1852/asilano/autocorrect-allow-prompt-default-yes-v1

To fetch this version to local tag pr-1852/asilano/autocorrect-allow-prompt-default-yes-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1852/asilano/autocorrect-allow-prompt-default-yes-v1

Copy link

gitgitgadget bot commented Jan 15, 2025

On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this):

On Wed, Jan 15, 2025, at 10:36, Chris Howlett via GitGitGadget wrote:
> From: Chris Howlett <[email protected]>
>
> The help.autocorrect functionality is really useful, saving frustration
> when a dev fat-fingers a command, and git has a pretty good idea what
> was originally intended. The config settings are a nice selection, with
> "prompt" asking the user to confirm that they want to run the assumed
> command.
>
> However, with "prompt", the choice defaults to "No" - that is, hitting
> return will _not_ run the command. For me at least, if git is confident
> it knows which command I wanted, it's usually right, and the golden path
> would be to run the command.
>
> Therefore this patch adds "prompt-yes" as a counterpart config setting
> for help.autocorrect, which does the same as "prompt", but defaults to
> "Yes" - hitting return will run the assumed command.
>
> I have not added any tests because the test suite doesn't have any tests
> (that I could find) for the "prompt" behaviour - I'm assuming this is
> because it's hard/impossible to simulate the interactive terminal prompt
>
> Signed-off-by: Chris Howlett <[email protected]>

This seems to conflict with the patch “help: interpret boolean string
values for help.autocorrect” which is in `seen`.  The latest version (I
don’t know what version is applied right now):

https://lore.kernel.org/git/[email protected]/

Copy link

gitgitgadget bot commented Jan 15, 2025

User "Kristoffer Haugsbakk" <[email protected]> has been added to the cc: list.

Copy link

gitgitgadget bot commented Jan 15, 2025

On the Git mailing list, Chris Howlett wrote (reply to this):

On Wed, 15 Jan 2025 at 09:51, Kristoffer Haugsbakk
<[email protected]> wrote:
>
> On Wed, Jan 15, 2025, at 10:36, Chris Howlett via GitGitGadget wrote:
> > From: Chris Howlett <[email protected]>
> >
> > The help.autocorrect functionality is really useful, saving frustration
> > when a dev fat-fingers a command, and git has a pretty good idea what
> > was originally intended. The config settings are a nice selection, with
> > "prompt" asking the user to confirm that they want to run the assumed
> > command.
> >
> > However, with "prompt", the choice defaults to "No" - that is, hitting
> > return will _not_ run the command. For me at least, if git is confident
> > it knows which command I wanted, it's usually right, and the golden path
> > would be to run the command.
> >
> > Therefore this patch adds "prompt-yes" as a counterpart config setting
> > for help.autocorrect, which does the same as "prompt", but defaults to
> > "Yes" - hitting return will run the assumed command.
> >
> > I have not added any tests because the test suite doesn't have any tests
> > (that I could find) for the "prompt" behaviour - I'm assuming this is
> > because it's hard/impossible to simulate the interactive terminal prompt
> >
> > Signed-off-by: Chris Howlett <[email protected]>
>
> This seems to conflict with the patch “help: interpret boolean string
> values for help.autocorrect” which is in `seen`.  The latest version (I
> don’t know what version is applied right now):
>
> https://lore.kernel.org/git/[email protected]/

That's unsurprising, as I was inspired to add this option after
reading that committer's blog post on help.autocorrect -
https://blog.gitbutler.com/why-is-git-autocorrect-too-fast-for-formula-one-drivers/

I'm happy to wait for their patch to be merged, then rebase and rework
against it, if that seems the most sensible option? Presumably I'll
have to monitor the mailing list to learn when that happens? This is
my first patch to git, so I'm not sure of process.

Copy link

gitgitgadget bot commented Jan 15, 2025

On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this):

On Wed, Jan 15, 2025, at 11:21, Chris Howlett wrote:
> On Wed, 15 Jan 2025 at 09:51, Kristoffer Haugsbakk
> > [snip]
>
> That's unsurprising, as I was inspired to add this option after
> reading that committer's blog post on help.autocorrect -
> https://blog.gitbutler.com/why-is-git-autocorrect-too-fast-for-formula-one-drivers/
>
> I'm happy to wait for their patch to be merged, then rebase and rework
> against it, if that seems the most sensible option? Presumably I'll
> have to monitor the mailing list to learn when that happens? This is
> my first patch to git, so I'm not sure of process.

You can keep an eye on the “What's cooking” emails.[1]  The latest
one mentions this other topic as branch `sc/help-autocorrect-one`
(that’s Scott Chacon’s initals followed by the topic name).

If you want to wait for that one to get merged:

• Wait until you see such an email with this “status” under it:

       Will merge to 'master'.

  Which means that it will be merged to `master` soon.

(Or maybe run

    git branch --remote --contains=origin/sc/help-autocorrect-one \
    | grep master

from time to time (I don’t know, I’ve never had to do that).)

Or else you could build on top of it.  That’s more advanced though.
Not something I’ve done myself.[2]

† 1: Latest: https://lore.kernel.org/git/[email protected]/
† 2: See `Documentation/SubmittingPatches under “select few topic
    branches that are” for how to depend on in-flight topics

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

Successfully merging this pull request may close these issues.

2 participants