On Mon, Apr 9, 2018 at 12:16 AM, Junio C Hamano <gits...@pobox.com> wrote: > > I can connect "because it deals only with remotes" and "access to > the object may fail", but I wonder if we can clarify the former a > bit better to make it easier to understand for those who are not yet > familiar with Git. > > Keys like `committerdate` that require access to the object will > not work [***HOW??? Do we get a segfault or something???***] for > refs whose objects have not yet been fetched from the remote. > > I added "for refs whose objects have not yet been fetched", whose > explicit-ness made "will not work" to be an OK expression, but > without it I would have suggested to rephrase it to "may not work". > > This is a tangent but I suspect that the description of --sort in > git-for-each-ref documentation is wrong on the use of multiple > options, or I am misreading parse_opt_ref_sorting(), which I think > appends later keys to the end of the list, and compare_refs() which > I think yields results when an earlier key in the last decides two > are not equal and ignores the later keys. The commit that added the > description was c0f6dc9b ("git-for-each-ref.txt: minor > improvements", 2008-06-05), and I think even back then the code in > builtin-for-each-ref.c appended later keys at the end, and it seems > nobody complained, so it is possible I am not reading the code right > before fully adjusting to timezone change ;-)
Updated the docs. > > It is not too big a deal, but I find that liberal sprinkling of > UNLEAK() is somewhat unsightly. From the code we leave with this > change, it is clear what we'd need to do when we make the code fully > leak-proof (i.e. we'd have a several lines to free resources held by > these two variables here, and then UNLEAK() that appear earlier in > the function will become a "goto cleanup;" after setting appropriate > value to "status"), so let's not worry about it. Removed the "extra" unleak annotations within the if-return blocks, but kept them at the end. > Please do *NOT* step outside the pair of single quotes that protects > the body of the test scriptlet and execute commands like "git > rev-parse". These execute in order to prepare the command line > argument strings BEFORE test_expect_success can run (or the test > framework decides if the test will be skipped via GIT_TEST_SKIP). > > Instead do it like so: > > cat >expect <<-EOF && > $(git rev-parse mark) refs/tags/mark > $(git rev-parse mark1.1) refs/tags/mark1.1 > $(git rev-parse mark1.2) refs/tags/mark1.2 > $(git rev-parse mark1.10) refs/tags/mark1.10 > EOF > > I.e. the end token for << that is not quoted allows $var and $(cmd) > to be substituted. > > Same comment applies throughout the remainder of this file. Ah, thanks! I was looking for some way to allow the values to expand within the proper test context. So turns out '<<-\EOF' vs '<<-EOF' makes all the difference :) > Other than that, this patch was a very pleasant read. Thanks. Thank you!