Re: [PATCH 1/3] t5543: never report what we do not push
- From
Jiang Xin <worldhello.net@gmail.com>
- Date
- Mar 26, 2020, 02:25 UTC
- Message-ID
- <CANYiYbEQPUu5OyrGkmM7SVuX-edffn_5n5ywXv6D2Xh0=O5raA@mail.gmail.com>
- In-Reply-To
- <xmqqa744xyoo.fsf@gitster.c.googlers.com>
Junio C Hamano <gitster@pobox.com> 于2020年3月25日周三 下午11:05写道：
Show 8 quoted lines
>
> Jiang Xin <worldhello.net@gmail.com> writes:
>
> > +format_git_output () {
>
> Unless this helper is able to take any git output and massage,
> please describe what kind of git output it is meant to handle.
>Wil use "cut_status_report_and_tidy" as the function name.
Show 12 quoted lines
> Also, "format" does not tell anything to the readers why it wants to
> transform its input or what its output is supposed to look like. It
> does not help readers and future developers.
>
> > + awk '/^(To| !) / {print}' | \
> > + sed \
> > + -e "s/ *\$//g" \
>
> What's the point of /g? You are anchoring the pattern (i.e. one or
> more SP) to the right end of the line, so it's not like it would
> transform "a b c " into "abc". Also it would be sufficient to
> say "zero or more" and that would be shorter, I think.I want to remove the trailing spaces in git output. I don't know why there are trailing spaces in each "remote: " message. But there is not trailing spaces in status report message, so I will remote it in next reroll.
Show 19 quoted lines
> > > + -e "s/'/\"/g" > > It is unclear what this thing is for. If the output from a git > subcommand you are munging with the helper says <don't>, this will > turn it into <don"t>, presumably before comparing it with the > expected output you'd literally prepare in the test script. Are the > git subcommands whose output you are munging unstable and sometimes > use single and sometimes use double quotes? > > If not, if you used single quotes when preparing the expected > output, wouldn't you be able to do without this? > > Is it because you'd have the code that prepares the expected output > inside a sq pair (because it is in test_expect_thing), and it is > cumbersome to write a literal single quote? If that is the reason, > that is understandable, but I think readers deserve some comments > explaining why these transformations are done. Please do not waste > readers' time.
To prepare expect message, sometime I have to escape the single quote like this
cat >expect <<-EOF
error: failed to push some refs to '"'"'../upstream'"'"'
EOFIt's boring to type '"'"' instead of a double quote.
Show 11 quoted lines
>
> It looks wasteful to pipe awk output to sed. I wonder if something
> along the lines of
>
> sed -ne "/^\(To\| !\) /{
> s/ *\$//
> s/'/\"/g
> p
> }"
>
> would do the same thing with a single tool.That's better.
Show 14 quoted lines
> > +# References in upstream : master(1) one(1) foo(1) > > +# References in workbench: master(2) foo(1) two(2) bar(2) > > +# Atomic push : master(2) two(2) bar(2) > > +test_expect_failure 'atomic push reports (reject by update hook)' ' > > + mk_repo_pair && > > + ( > > + cd workbench && > > + # Keep constant output. > > + git config core.abbrev 7 && > > This '7' is "use at least seven hexdigits", so it does not give any > guarantee that your output will be stable. Depending on chance, > some objects may be shown using 8 or more---is our "munge output > before comparison" helper prepared for such a case?
Will use sed -e 's/ */ /g' on output message to make it stable.
Show 16 quoted lines
> > > + test_commit one && > > + git branch foo && > > + git push up master one foo && > > + git tag -d one > > + ) && > > + ( > > + mkdir -p upstream/.git/hooks && > > + cat >upstream/.git/hooks/update <<-EOF && > > + #!/bin/sh > > + > > + if test "\$1" = "refs/heads/bar" > > + then > > + echo >2 "Pusing to branch bar is prohibited" > > Meant to redirect to the standard error stream, not a file "2"?
It's a typo, should be "echo >&2".