git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v5 4/4] commit: Add commit.verbose configuration

From
Caleb Thompson <caleb@calebthompson.io>
Date
Jun 16, 2014, 20:10 UTC
Message-ID
<20140616201037.GA37953@sirius.local>
In-Reply-To
<xmqqzjhcpqju.fsf@gitster.dls.corp.google.com>
On Mon, Jun 16, 2014 at 01:06:45PM -0700, Junio C Hamano wrote:
Show 54 quoted lines
> Caleb Thompson <caleb@calebthompson.io> writes:
>
> > On Fri, Jun 13, 2014 at 10:48:55AM -0700, Junio C Hamano wrote:
> >> Caleb Thompson <caleb@calebthompson.io> writes:
> >>
> >> > diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
> >> > index 35a4d06..402d6a1 100755
> >> > --- a/t/t7507-commit-verbose.sh
> >> > +++ b/t/t7507-commit-verbose.sh
> >> > @@ -7,6 +7,10 @@ write_script check-for-diff <<-'EOF'
> >> >		exec grep '^diff --git' "$1"
> >> >  EOF
> >> >
> >> > +write_script check-for-no-diff <<-'EOF'
> >> > +	exec grep -v '^diff --git' "$1"
> >> > +EOF
> >>
> >> This lets grep show all lines that are not "diff --git" in the
> >> input, and as usual grep exits success if it has any line in the
> >> output.
> >>
> >>     $ grep -v '^diff --git' <<\EOF ; echo $?
> >>     diff --git
> >>     a
> >>     EOF
> >>     a
> >>     0
> >>     $ exit
> >>
> >> What are we testing, exactly?
> >
> > Good catch. It worked when I switched check-for-diff from
> > check-for-no-diff, but I didn't try to make check-for-no-diff fail
> > independently, so I apologize.
>
> No need to apologize at all.  None of us (including this reviewer)
> is perfect and that is why we review patches by each other.
>
> > This version removes the the beginning of a line starting with
> > "diff --git" from the string,...
>
> Again, what are we testing, exactly?
>
> We do not want to see "^diff --git" in the output file, in other
> words, we want to make sure "^diff --git" does not appear in the
> output.
>
> So
>
>         write_script check-for-no-diff <<-\EOF
>         ! grep '^diff --git' "$@"
>	EOF
>
> should be the most natural way to express what we are testing, no?

I did consider that. The reason I didn't propose that is that it doesn't catch the unlikely case that the $1 only contains a "diff --git" line or that $1 is empty.

Those are rather unreasonable concerns, so I'm happy to use the much more readable version as you propose.

Caleb Thompson
Previous: Junio C HamanoNext: Junio C Hamano
Message 23 of 28 in “commit: Add commit.verbose configuration”
  1. 0/4 commit: Add commit.verbose configurationCaleb Thompson, Jun 12, 2014
  2. 1/4 commit test: Use test_config instead of git-configCaleb Thompson, Jun 12, 2014
  3. 2/4 commit test: Use write_scriptCaleb Thompson, Jun 12, 2014
  4. Jeff KingJun 13, 2014
  5. Caleb ThompsonJun 13, 2014
  6. Jeff KingJun 13, 2014
  7. 3/4 commit test: test_set_editor in each testCaleb Thompson, Jun 12, 2014
  8. Jeff KingJun 13, 2014
  9. Caleb ThompsonJun 13, 2014
  10. Jakub NarębskiJun 13, 2014
  11. Caleb ThompsonJun 13, 2014
  12. Jakub NarębskiJun 13, 2014
  13. Jeff KingJun 13, 2014
  14. Junio C HamanoJun 13, 2014
  15. Jeff KingJun 13, 2014
  16. Caleb ThompsonJun 16, 2014
  17. Junio C HamanoJun 16, 2014
  18. 4/4 commit: Add commit.verbose configurationCaleb Thompson, Jun 12, 2014
  19. Junio C HamanoJun 13, 2014
  20. Caleb ThompsonJun 16, 2014
  21. Caleb ThompsonJun 16, 2014
  22. Junio C HamanoJun 16, 2014
  23. Caleb ThompsonJun 16, 2014
  24. Junio C HamanoJun 16, 2014
  25. Jeremiah MahlerJun 12, 2014
  26. Caleb ThompsonJun 13, 2014
  27. Jeremiah MahlerJun 14, 2014
  28. Junio C HamanoJun 16, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.