completion (zsh): fix misleading install location - #609

/allow durantschoon

Please see: #137 and close it if this is accepted (sorry for creating this separately, maybe I should have rebased my original).

Ideally, #137 should have been updated, but I see that it is marked with "unkown repository", so I would wager a guess that you deleted your fork in the meantime, and that you would have no way to update it even if you wanted to.

So please close that PR yourself: it is yours, so you can easily do that house-cleaning yourself.

Also, allow me to comment a bit more on the form of the change before telling you how you can contribute it to the Git mailing list.

Indicating that _git is a folder and not a file might save a new zsh user a world of pain. I learned this one the hard way.

Signed-off-by: B. Durant Schoon [email protected]

That is an okay commit message, but you use it as a cover letter. Please move this description into the actual commit message (including the sign off).

Although I have to admit that my fingers itch to suggest improvements (apart from wrapping the text to <=76 columns per line, which is required by the Git project): how about this:

When the code comment in the zsh completion suggests that this file
should be copied to `~/.zsh`, many users might be misled to believe that
this refers to a file location. But it refers to a directory, and won't
work when it is a file.

Let's just add a slash, to make it abundantly clear that this must be a
directory.

The first line of the commit message, however, should be a super-short summary of the change, and not necessarily so much be about the implementation as about the motivation that led you to implement the change.

In your case, I would suggest something like "completion (zsh): fix misleading install location".

Now, as promised, the suggestion how to submit this to the Git mailing list for review. You could use submitGit with this here PR. If that does not work for you, or if you do not want to use it, you could use GitGitGadget instead. This comment of yours makes me think that you want to use GitGitGadget:

/allow durantschoon

However, for technical reasons, GitGitGadget currently requires a new Pull Request to be opened at https://github.com/gitgitgadget/git (you can use the same branch, actually, and do not need to push a new one).

Also, you cannot add yourself to the list of people allowed to use GitGitGadget, that would defeat the purpose of this light-weight spam preventing measure! ;-)

Oh, and I totally forgot to mention: please squash the two commits into one. I don't think it is worth to keep those changes apart. Maybe the re-wrapping is not even necessary? Maybe you thought you had to wrap the text to 76 columns per line? That only applies to commit messages, the text in the files is still usually wrapped at 80 columns per line, as per the Linux kernel's custom.

/submit

Sorry. I'm having trouble figuring out how to squash those commits...

Commits are hopefully squashed now and patch has been sent via submitGit. Thanks for your help!

Commits are hopefully squashed now

Yes.

But the commit message still is too short: 5a0a37d

I see that you used the commit message that I proposed as PR description instead. The problem with that is that the PR description won't make it into the commit history. The commit message will.

patch has been sent via submitGit.

I do not see it at https://public-inbox.org/git/. Do you?

I've made another attempt with 7e72d69 although submitgit is crashing for me at the moment.

Thank you for your help!

I've made another attempt with 7e72d69

Looks good to me!

submitgit is crashing for me at the moment.

@rtyley any ideas?

submitgit is working again, but it warns me:

This PR is based off master, so in general it should be a new feature, not a bugfix.

I see I need to register my email address with Amazon SES to complete submission.

@durantschoon sent this commit (74583d8...7e72d69) as a patch to the mailing list with submitGit - here on public-inbox, MARC

@durantschoon have you seen https://public-inbox.org/git/[email protected]/?

I think that those are two good suggestions, and once you amended the commit and force-pushed, you can use submitGit to send another iteration that should then be picked up, I think.

@durantschoon gentle ping?

@durantschoon sent this commit (4c86140...8510768) as a patch to the mailing list with submitGit - here on public-inbox, MARC

Maybe gitgitgadget#367 supersedes this? (There was a suggestion on the mailing list that still waits for an answer...)

I believe that c1ec35d addresses this.