[PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[Ted Pavlic]

Third in a series of patches that make bash completions more robust to
different interactive shell configurations and editors.

[PATCH 3/3] Adds a #!bash to the top of bash completions so that editors 
can recognize
  it as a bash script. Also adds a few simple comments above commands that
  take arguments. The comments are meant to remind editors of potential
  problems that can occur when the script is sourced on systems with "set
  -u."

Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
---
  contrib/completion/git-completion.bash |   15 +++++++++++++++
  1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 201f9a6..f8b845a 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1,3 +1,4 @@
+#!bash
  #
  # bash completion support for core Git.
  #
@@ -50,6 +51,8 @@ case "$COMP_WORDBREAKS" in
  *)   COMP_WORDBREAKS="$COMP_WORDBREAKS:"
  esac

+# __gitdir accepts 0 or 1 arguments (i.e., location)
+# returns location of .git repo
  __gitdir ()
  {
  	if [ -z "${1-}" ]; then
@@ -67,6 +70,8 @@ __gitdir ()
  	fi
  }

+# __git_ps1 accepts 0 or 1 arguments (i.e., format string)
+# returns text to add to bash PS1 prompt (includes branch name)
  __git_ps1 ()
  {
  	local g="$(git rev-parse --git-dir 2>/dev/null)"
@@ -119,6 +124,7 @@ __git_ps1 ()
  	fi
  }

+# __gitcomp_1 requires 2 arguments
  __gitcomp_1 ()
  {
  	local c IFS=' '$'\t'$'\n'
@@ -131,6 +137,8 @@ __gitcomp_1 ()
  	done
  }

+# __gitcomp accepts 1, 2, 3, or 4 arguments
+# generates completion reply with compgen
  __gitcomp ()
  {
  	local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -150,6 +158,7 @@ __gitcomp ()
  	esac
  }

+# __git_heads accepts 0 or 1 arguments (to pass to __gitdir)
  __git_heads ()
  {
  	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -168,6 +177,7 @@ __git_heads ()
  	done
  }

+# __git_tags accepts 0 or 1 arguments (to pass to __gitdir)
  __git_tags ()
  {
  	local cmd i is_hash=y dir="$(__gitdir "${1-}")"
@@ -186,6 +196,7 @@ __git_tags ()
  	done
  }

+# __git_refs accepts 0 or 1 arguments (to pass to __gitdir)
  __git_refs ()
  {
  	local i is_hash=y dir="$(__gitdir "${1-}")"
@@ -218,6 +229,7 @@ __git_refs ()
  	done
  }

+# __git_refs2 requires 1 argument (to pass to __git_refs)
  __git_refs2 ()
  {
  	local i
@@ -226,6 +238,7 @@ __git_refs2 ()
  	done
  }

+# __git_refs_remotes requires 1 argument (to pass to ls-remote)
  __git_refs_remotes ()
  {
  	local cmd i is_hash=y
@@ -470,6 +483,7 @@ __git_aliases ()
  	done
  }

+# __git_aliased_command requires 1 argument
  __git_aliased_command ()
  {
  	local word cmdline=$(git --git-dir="$(__gitdir)" \
@@ -482,6 +496,7 @@ __git_aliased_command ()
  	done
  }

+# __git_find_subcommand requires 1 argument
  __git_find_subcommand ()
  {
  	local word subcommand c=1
-- 
1.6.1.87.g15624

Re: [PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[Shawn O. Pearce]

Ted Pavlic <ted@tedpavlic.com> wrote:
>
> Third in a series of patches that make bash completions more robust to
> different interactive shell configurations and editors.
>
> [PATCH 3/3] Adds a #!bash to the top of bash completions so that editors  
> can recognize
>  it as a bash script. Also adds a few simple comments above commands that
>  take arguments. The comments are meant to remind editors of potential
>  problems that can occur when the script is sourced on systems with "set
>  -u."

Aside from the message format... OK.  The message really should
have looked like this from an mbox point of view:

	From: Ted Pavlic <ted@tedpavlic.com>
	To: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>
	Cc: "Shawn O. Pearce" <spearce@spearce.org>
	Bcc: 
	Subject: [PATCH 3/3] bash-completion: Add internal function documentation

	Slightly document the internal functions of the bash
	completion package, so callers are more easily able to
	determine the expected arguments.

	Signed-off-by: Ted Pavlic <ted@tedpavlic.com>
	---

	 Third in a series to improve the bash completion package,
	 so it sucks less.

	 contrib/completion/git-completion.bash |   15 +++++++++++++++
	 1 files changed, 15 insertions(+), 0 deletions(-)

	diff --git a/contrib/completion/git-completion.bash  

See how the stuff that doesn't matter to the commit message itself
goes after the "---" line?  And how the subject is a niceshort, one
line summary of the module impacted and the change?  These show up in
gitk and git shortlog, and thus in the "What's changed in git.git"
newsletters Junio publishes.  Its important that the subject be
really short and sweet.  You can put more detail above the "---"
line, and it will be included in the commit when Junio applies it.

This is all based on the formatting at the time of commit.
Anything up to the first "\n\n" in a commit message goes into the
email subject line.  The rest goes into the email body, but above the
"---" line.  You can then edit the buffer before sending to insert
non-commit message text after the "---" and before the diff stat.

You can include my Ack'd by line below your Signed-off-by when you
resend it.

Acked-by: Shawn O. Pearce <spearce@spearce.org>

> ---
>  contrib/completion/git-completion.bash |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)

-- 
Shawn.

Re: [PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[Boyd Stephen Smith Jr.]

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

On Tuesday 2009 January 13 10:45:18 Shawn O. Pearce wrote:
>See [...] how the subject is a niceshort, one
>line summary of the module impacted and the change?

My rule for this is absolutely no more than 80 characters.  Generally, you 
wouldn't want more than 60 or so, since it is used as the Subject: header and 
generally has some prefix added.

As shown says, details can go in the rest of the commit message.  If you are 
using more than 60-80 characters even without details, you might think about 
splitting the patch.

>This is all based on the formatting at the time of commit.
>Anything up to the first "\n\n" in a commit message goes into the
>email subject line.

IIRC, multiple "-m" options to "git commit" will be separated by "\n\n", so 
that's one way to do it if you don't like your $EDITOR for some reason.
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss@iguanasuicide.net                     ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.net/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[Adeodato Simó]

* Boyd Stephen Smith Jr. [Tue, 13 Jan 2009 14:03:11 -0600]:

> On Tuesday 2009 January 13 10:45:18 Shawn O. Pearce wrote:
> >See [...] how the subject is a niceshort, one
> >line summary of the module impacted and the change?

> My rule for this is absolutely no more than 80 characters.

My rule for *all* of the commit message is "absolutely no more than 76
characters". With more than 76, `git log` wraps in a 80-column terminal.

Just my 2¢,

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
La música es de los que la quieren escuchar y de nadie más.
                -- Andrés Calamaro

Commit messages

[Teemu Likonen]

Adeodato Simó (2009-01-13 21:10 +0100) wrote:

> * Boyd Stephen Smith Jr. [Tue, 13 Jan 2009 14:03:11 -0600]:
>
>> My rule for this is absolutely no more than 80 characters.
>
> My rule for *all* of the commit message is "absolutely no more than 76
> characters". With more than 76, `git log` wraps in a 80-column terminal.

Here's my rule:


(add-to-list 'auto-mode-alist
             '("/\\.git/\\(COMMIT\\|TAG\\)_EDITMSG\\'" .
               vcs-message-mode))

(define-derived-mode vcs-message-mode text-mode "VCS-message"
  "Major mode for editing commit and tag messages." 
  (auto-fill-mode 1)
  (set (make-local-variable 'tab-stop-list)
       (number-sequence 4 100 4))
  (setq indent-tabs-mode nil
        fill-column 72
        truncate-lines t))

Re: Commit messages

[Ted Pavlic]

That rule could be modified to support .stgit-edit.txt as well.

> (add-to-list 'auto-mode-alist
>               '("/\\.git/\\(COMMIT\\|TAG\\)_EDITMSG\\'" .
>                 vcs-message-mode))

-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

Re: [PATCH 3/3] Adds a #!bash to the top of bash completions so that editors can recognize, it as a bash script. Also adds a few simple comments above commands that, take arguments. The comments are meant to remind editors of potential, problems that can occur when the script is sourced on systems with "set, -u."

[Markus Heidelberg]

Adeodato Simó, 13.01.2009:
> * Boyd Stephen Smith Jr. [Tue, 13 Jan 2009 14:03:11 -0600]:
> 
> > On Tuesday 2009 January 13 10:45:18 Shawn O. Pearce wrote:
> > >See [...] how the subject is a niceshort, one
> > >line summary of the module impacted and the change?
> 
> > My rule for this is absolutely no more than 80 characters.
> 
> My rule for *all* of the commit message is "absolutely no more than 76
> characters". With more than 76, `git log` wraps in a 80-column terminal.

What about the 50 character limit proposed in the documentation
(git-commit, gittutorial, user-manual)?

At the beginning I tried to fulfil this limit, but often it's not easy.
So should it be adjusted to a slightly higher value in the documentation
or even split into a recommended limit (e.g. 50) and a recommended
absolute maximum (e.g. 76)? Hmm, the split wouldn't make sense, I think.

Markus

Commit messages

[Paul Eggleton]

Hi folks,

I've been going through the commits in preparation for our release notes, and 
unfortunately what I'm seeing is that the number of empty or incomplete commit 
messages has noticeably increased over previous releases. Commit messages are 
vitally important to providing a readable history, not just in support of 
preparing release notes, but also if you're trying to track down a regression 
or any other reason you might want to go back and find out what has been done 
and most importantly *why*.

Here are our commit message guidelines, which have been in place for some 
time:

  http://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines

I know everyone's busy, but please take 5 minutes to read these if you haven't 
recently and try to keep them in mind when writing commit messages. No commit 
is too trivial for a complete commit message.

Thanks,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: Commit messages

[akuster808]

Paul,


On 10/09/2016 05:35 PM, Paul Eggleton wrote:
> Hi folks,
>
> I've been going through the commits in preparation for our release notes, and
> unfortunately what I'm seeing is that the number of empty or incomplete commit
> messages has noticeably increased over previous releases. Commit messages are
> vitally important to providing a readable history, not just in support of
> preparing release notes, but also if you're trying to track down a regression
> or any other reason you might want to go back and find out what has been done
> and most importantly *why*.
Also helps in determining if a commit needs to be backported to stable 
branches.

> Here are our commit message guidelines, which have been in place for some
> time:
>
>    http://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
>
> I know everyone's busy, but please take 5 minutes to read these if you haven't
> recently and try to keep them in mind when writing commit messages. No commit
> is too trivial for a complete commit message.
Agreed. We also need more folks reviewing incoming patches to help 
spread the load.

The other options is to should start rejecting commits that do comply to 
the guidelines.

Thanks for bringing this up.


Regards,
Armin
>
> Thanks,
> Paul
>

Re: Commit messages

[Paul Eggleton]

Hi Armin,

On Sun, 09 Oct 2016 20:29:39 akuster808 wrote:
> On 10/09/2016 05:35 PM, Paul Eggleton wrote:
> > I've been going through the commits in preparation for our release notes,
> > and unfortunately what I'm seeing is that the number of empty or
> > incomplete commit messages has noticeably increased over previous
> > releases. Commit messages are vitally important to providing a readable
> > history, not just in support of preparing release notes, but also if
> > you're trying to track down a regression or any other reason you might
> > want to go back and find out what has been done and most importantly
> > *why*.
> 
> Also helps in determining if a commit needs to be backported to stable
> branches.

Indeed.
 
> > Here are our commit message guidelines, which have been in place for some
> > 
> > time:
> >    http://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
> > 
> > I know everyone's busy, but please take 5 minutes to read these if you
> > haven't recently and try to keep them in mind when writing commit
> > messages. No commit is too trivial for a complete commit message.
> 
> Agreed. We also need more folks reviewing incoming patches to help
> spread the load.

That is indeed true.

> The other options is to should start rejecting commits that do comply to
> the guidelines.

Actually I forgot to mention one thing in my email - it's been a long time 
coming, but we should soon have a set of tests automatically run on incoming 
patches, which would immediately flag these kinds of basic things up before a 
human needs to look at the patch. More to come on that soon.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: Commit messages

[Paul Menzel]

Dear Christian, dear Daniel, dear Alex,


Am 23.03.22 um 16:32 schrieb Christian König:
> Am 23.03.22 um 16:24 schrieb Daniel Stone:
>> On Wed, 23 Mar 2022 at 15:14, Alex Deucher <alexdeucher@gmail.com> wrote:
>>> On Wed, Mar 23, 2022 at 11:04 AM Daniel Stone <daniel@fooishbar.org> 
>>> wrote:
>>>> That's not what anyone's saying here ...
>>>>
>>>> No-one's demanding AMD publish RTL, or internal design docs, or
>>>> hardware specs, or URLs to JIRA tickets no-one can access.
>>>>
>>>> This is a large and invasive commit with pretty big ramifications;
>>>> containing exactly two lines of commit message, one of which just
>>>> duplicates the subject.
>>>>
>>>> It cannot be the case that it's completely impossible to provide any
>>>> justification, background, or details, about this commit being made.
>>>> Unless, of course, it's to fix a non-public security issue, that is
>>>> reasonable justification for eliding some of the details. But then
>>>> again, 'huge change which is very deliberately opaque' is a really
>>>> good way to draw a lot of attention to the commit, and it would be
>>>> better to provide more detail about the change to help it slip under
>>>> the radar.
>>>>
>>>> If dri-devel@ isn't allowed to inquire about patches which are posted,
>>>> then CCing the list is just a façade; might as well just do it all
>>>> internally and periodically dump out pull requests.
>>> I think we are in agreement. I think the withheld information
>>> Christian was referring to was on another thread with Christian and
>>> Paul discussing a workaround for a hardware bug:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Famd-gfx%2Fmsg75908.html&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7C6a3f2815d83b4872577008da0ce1347a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637836458652370599%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=QtNB0XHMhTgH%2FNHMwF23Qn%2BgSdYyHJSenbpP%2FHG%2BkxE%3D&amp;reserved=0 

(Thank you Microsoft for keeping us safe.)

I guess it proves, how assuming what other people should know/have read, 
especially when crossing message threads, is causing confusion and 
misunderstandings.

>> Right, that definitely seems like some crossed wires. I don't see
>> anything wrong with that commit at all: the commit message and a
>> comment notes that there is a hardware issue preventing Raven from
>> being able to do TMZ+GTT, and the code does the very straightforward
>> and obvious thing to ensure that on VCN 1.0, any TMZ buffer must be
>> VRAM-placed.

My questions were:

> Where is that documented, and how can this be reproduced? 

Shouldn’t these be answered by the commit message? In five(?) years, 
somebody, maybe even with access to the currently non-public documents, 
finds a fault in the commit, and would be helped by having an 
document/errata number where to look at. To verify the fix, the 
developer would need a method to produce the error, so why not just 
share it?

Also, I assume that workarounds often come with downsides, as otherwise 
it would have been programmed like this from the beginning, or instead 
of “workaround” it would be called “improvement”. Shouldn’t that also be 
answered?

So totally made-up example:

Currently, there is a graphics corruption running X on system Y. This is 
caused by a hardware bug in Raven ASIC (details internal document 
#NNNN/AMD-Jira #N), and can be worked around by [what is in the commit 
message].

The workaround does not affect the performance, and testing X shows the 
error is fixed.

>> This one, on the other hand, is much less clear ...
> 
> Yes, completely agree. I mean a good bunch of comments on commit 
> messages are certainly valid and we could improve them.

That’d be great.

> But this patch here was worked on by both AMD and Intel developers. 
> Where both sides and I think even people from other companies perfectly 
> understands why, what, how etc...
> 
> When now somebody comes along and asks for a whole explanation of the 
> context why we do it then that sounds really strange to me.

The motivation should be part of the commit message. I didn’t mean 
anyone to rewrite buddy memory allocator Wikipedia article [1]. But the 
commit message at hand for switching the allocator is definitely too terse.


Kind regards,

Paul


[1]: https://en.wikipedia.org/wiki/Buddy_memory_allocation