{"thread":{"id":"28216","subject":"[PATCH] replace: List replacement along with the object","startedAt":"2011-08-25T14:39:44Z","lastAt":"2011-08-27T14:07:28Z","messageCount":8,"participants":["Michael J Gruber","Christian Couder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174229","messageId":"bae5a8f7d30417864d972390f9f6b4470cf4e5bf.1314283118.git.git@drmicha.warpmail.net","threadId":"28216","inReplyTo":null,"subject":"[PATCH] replace: List replacement along with the object","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-08-25T14:39:44Z","receivedAt":"2011-08-25T14:39:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"The documentation could be misunderstood as if \"git replace -l\" lists\nthe replacements of the specified objects. Currently, it lists the\nreplaced objects.\n\nChange the output to the form \"<object> <replacement>\" so that there is\nan easy way to find the replacement, besides the more difficult to find\ngit show-ref $(git replace -l).\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nNoted in passing while testing decorations.\n---\n Documentation/git-replace.txt |    2 +-\n builtin/replace.c             |    2 +-\n t/t6050-replace.sh            |    8 ++++----\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt\nindex 17df525..9bf3ff8 100644\n--- a/Documentation/git-replace.txt\n+++ b/Documentation/git-replace.txt\n@@ -58,7 +58,7 @@ OPTIONS\n \n -l <pattern>::\n \tList replace refs for objects that match the given pattern (or\n-\tall if no pattern is given).\n+\tall if no pattern is given) in the form \"<object> <replacement>\".\n \tTyping \"git replace\" without arguments, also lists all replace\n \trefs.\n \ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex fe3a647..f8c5a9f 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -26,7 +26,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,\n \tconst char *pattern = cb_data;\n \n \tif (!fnmatch(pattern, refname, 0))\n-\t\tprintf(\"%s\\n\", refname);\n+\t\tprintf(\"%s %s\\n\", refname, sha1_to_hex(sha1));\n \n \treturn 0;\n }\ndiff --git a/t/t6050-replace.sh b/t/t6050-replace.sh\nindex 5c87f28..665b308 100755\n--- a/t/t6050-replace.sh\n+++ b/t/t6050-replace.sh\n@@ -119,10 +119,10 @@ test_expect_success 'repack, clone and fetch work' '\n '\n \n test_expect_success '\"git replace\" listing and deleting' '\n-     test \"$HASH2\" = \"$(git replace -l)\" &&\n-     test \"$HASH2\" = \"$(git replace)\" &&\n+     test \"$HASH2 $R\" = \"$(git replace -l)\" &&\n+     test \"$HASH2 $R\" = \"$(git replace)\" &&\n      aa=${HASH2%??????????????????????????????????????} &&\n-     test \"$HASH2\" = \"$(git replace -l \"$aa*\")\" &&\n+     test \"$HASH2 $R\" = \"$(git replace -l \"$aa*\")\" &&\n      test_must_fail git replace -d $R &&\n      test_must_fail git replace -d &&\n      test_must_fail git replace -l -d $HASH2 &&\n@@ -137,7 +137,7 @@ test_expect_success '\"git replace\" replacing' '\n      test_must_fail git replace $HASH2 $R &&\n      git replace -f $HASH2 $R &&\n      test_must_fail git replace -f &&\n-     test \"$HASH2\" = \"$(git replace)\"\n+     test \"$HASH2 $R\" = \"$(git replace)\"\n '\n \n # This creates a side branch where the bug in H2\n-- \n1.7.6.845.gc3c05\n"},{"id":"174238","messageId":"CAP8UFD2Cr4UQjWa=pRvcqgyX_Ed+qjts=TujWRdyk4dUZsd_7Q@mail.gmail.com","threadId":"28216","inReplyTo":"bae5a8f7d30417864d972390f9f6b4470cf4e5bf.1314283118.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] replace: List replacement along with the object","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2011-08-25T16:29:45Z","receivedAt":"2011-08-25T16:29:45Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Aug 25, 2011 at 4:39 PM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n> The documentation could be misunderstood as if \"git replace -l\" lists\n> the replacements of the specified objects. Currently, it lists the\n> replaced objects.\n\nYou could just change the documentation to make it more explicit.\n\n> Change the output to the form \"<object> <replacement>\" so that there is\n> an easy way to find the replacement, besides the more difficult to find\n> git show-ref $(git replace -l).\n\nI shamelessly copied the \"-l <pattern>\" feature and the documentation\nfrom \"git tag\". If you just change the output of \"git replace -l\" it\nwill make the UI inconsistent between both commands.\n\nMaybe you could add a \"-L <pattern>\" feature to \"git replace\", \"git\ntag\" and \"git branch\" that would output \"<ref name> <ref content>\"?\n\nThanks,\nChristian.\n"},{"id":"174251","messageId":"7v1uw9l382.fsf@alter.siamese.dyndns.org","threadId":"28216","inReplyTo":"bae5a8f7d30417864d972390f9f6b4470cf4e5bf.1314283118.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] replace: List replacement along with the object","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-25T19:07:25Z","receivedAt":"2011-08-25T19:07:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> The documentation could be misunderstood as if \"git replace -l\" lists\n> the replacements of the specified objects. Currently, it lists the\n> replaced objects.\n\nSeeing that you had to change existing tests, I do not think this is an\nimprovement. The existing scripts can read the list of objects and find\nreplacement themselves (if they want to find that out, that is), no?\n"},{"id":"174303","messageId":"4E574D61.8050501@drmicha.warpmail.net","threadId":"28216","inReplyTo":"CAP8UFD2Cr4UQjWa=pRvcqgyX_Ed+qjts=TujWRdyk4dUZsd_7Q@mail.gmail.com","subject":"Re: [PATCH] replace: List replacement along with the object","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-08-26T07:38:09Z","receivedAt":"2011-08-26T07:38:09Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Christian Couder venit, vidit, dixit 25.08.2011 18:29:\n> On Thu, Aug 25, 2011 at 4:39 PM, Michael J Gruber\n> <git@drmicha.warpmail.net> wrote:\n>> The documentation could be misunderstood as if \"git replace -l\" lists\n>> the replacements of the specified objects. Currently, it lists the\n>> replaced objects.\n> \n> You could just change the documentation to make it more explicit.\n\nWell, sure. I just didn't find the current form that useful.\n\n>> Change the output to the form \"<object> <replacement>\" so that there is\n>> an easy way to find the replacement, besides the more difficult to find\n>> git show-ref $(git replace -l).\n> \n> I shamelessly copied the \"-l <pattern>\" feature and the documentation\n> from \"git tag\". If you just change the output of \"git replace -l\" it\n> will make the UI inconsistent between both commands.\n\nI don't think many people will expect consistency between branch and tag\non the one hand, and replace refs on the other hand. It requires the\nknowledge that a replacement is basically a lightweight tag stored in a\ndifferent namespace in refs/, which I would actually consider an\nimplementation detail.\n\n> Maybe you could add a \"-L <pattern>\" feature to \"git replace\", \"git\n> tag\" and \"git branch\" that would output \"<ref name> <ref content>\"?\n\nI'd use \"-v\" then if this is about consistency, because that *always*\nmeans \"verbose\", and migrate the misnamed \"git tag -v\"...\n\nJunio C Hamano venit, vidit, dixit 25.08.2011 21:07:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>> The documentation could be misunderstood as if \"git replace -l\" lists\n>> the replacements of the specified objects. Currently, it lists the\n>> replaced objects.\n> Seeing that you had to change existing tests, I do not think this is an\n> improvement. The existing scripts can read the list of objects and find\n> replacement themselves (if they want to find that out, that is), no?\n\nIf \"replace -l\" is considered fair game for scripts then the output\nshould probably not change, though I left the meaning of \"$1\" for each\nline of the output as is on purpose.\n\nBut, how would scripts find the replacement? rev-parse does not do it,\nrev-list does not do it, and using show-ref requires the user to know\nabout the actual implementation as refs under refs/replace.\n\nSeems that the doc change is the only option.\n\nMichael\n"},{"id":"174304","messageId":"0a88518db0b0db8f1a4a4deeebd6dffc2d603e74.1314345131.git.git@drmicha.warpmail.net","threadId":"28216","inReplyTo":"4E574D61.8050501@drmicha.warpmail.net","subject":"[PATCHv2] git-replace.txt: Clarify list mode","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-08-26T07:53:20Z","receivedAt":"2011-08-26T07:53:20Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Clarify that in list mode, \"git replace\" outputs the shortened ref\nnames, not their values.\n\nAlso, point to the difficult to find git show-ref $(git replace -l).\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n Documentation/git-replace.txt |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt\nindex 17df525..cd00837 100644\n--- a/Documentation/git-replace.txt\n+++ b/Documentation/git-replace.txt\n@@ -61,6 +61,13 @@ OPTIONS\n \tall if no pattern is given).\n \tTyping \"git replace\" without arguments, also lists all replace\n \trefs.\n++\n+Note that this lists the names of the replace refs, not their values\n+(not their replacements). You can get the latter like this, e.g.:\n+\n+------------------------------------------------\n+$ git show-ref $(git replace -l)\n+------------------------------------------------\n \n BUGS\n ----\n@@ -76,6 +83,7 @@ replaced by a commit).\n \n SEE ALSO\n --------\n+linkgit:git-show-ref[1]\n linkgit:git-tag[1]\n linkgit:git-branch[1]\n linkgit:git[1]\n-- \n1.7.6.845.gc3c05\n"},{"id":"174305","messageId":"CAP8UFD1Or8dPVBpAsOwge4o0Lpej2gTs7dQJM0eocbn7EiWVrQ@mail.gmail.com","threadId":"28216","inReplyTo":"4E574D61.8050501@drmicha.warpmail.net","subject":"Re: [PATCH] replace: List replacement along with the object","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2011-08-26T08:13:10Z","receivedAt":"2011-08-26T08:13:10Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Aug 26, 2011 at 9:38 AM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n> Christian Couder venit, vidit, dixit 25.08.2011 18:29:\n>> On Thu, Aug 25, 2011 at 4:39 PM, Michael J Gruber\n>> <git@drmicha.warpmail.net> wrote:\n>>> The documentation could be misunderstood as if \"git replace -l\" lists\n>>> the replacements of the specified objects. Currently, it lists the\n>>> replaced objects.\n>>\n>> You could just change the documentation to make it more explicit.\n>\n> Well, sure. I just didn't find the current form that useful.\n>\n>>> Change the output to the form \"<object> <replacement>\" so that there is\n>>> an easy way to find the replacement, besides the more difficult to find\n>>> git show-ref $(git replace -l).\n>>\n>> I shamelessly copied the \"-l <pattern>\" feature and the documentation\n>> from \"git tag\". If you just change the output of \"git replace -l\" it\n>> will make the UI inconsistent between both commands.\n>\n> I don't think many people will expect consistency between branch and tag\n> on the one hand, and replace refs on the other hand. It requires the\n> knowledge that a replacement is basically a lightweight tag stored in a\n> different namespace in refs/, which I would actually consider an\n> implementation detail.\n\nIt is an implementation detail, but anyway UI consistency is important\nand I would suggest the same behavior even if it was implemented in\nanother way.\nBy the way it would be nice to make \"git remote\" more similar to \"git\nbranch\", \"git tag\" and \"git replace\" while you are at it.\n\n>> Maybe you could add a \"-L <pattern>\" feature to \"git replace\", \"git\n>> tag\" and \"git branch\" that would output \"<ref name> <ref content>\"?\n>\n> I'd use \"-v\" then if this is about consistency, because that *always*\n> means \"verbose\", and migrate the misnamed \"git tag -v\"...\n\nYeah, but \"git branch -v\" is decribed like this:\n\n    Show sha1 and commit subject line for each head, along with\nrelationship to upstream branch (if any). If given twice, print the\nname of the upstream branch, as well.\n\nSo if you implement it in \"git replace\" and \"git tag\", you should at\nleast show the commit subject line too.\n\nThanks,\nChristian.\n"},{"id":"174326","messageId":"7v4o14f844.fsf@alter.siamese.dyndns.org","threadId":"28216","inReplyTo":"0a88518db0b0db8f1a4a4deeebd6dffc2d603e74.1314345131.git.git@drmicha.warpmail.net","subject":"Re: [PATCHv2] git-replace.txt: Clarify list mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-26T16:30:35Z","receivedAt":"2011-08-26T16:30:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Clarify that in list mode, \"git replace\" outputs the shortened ref\n> names, not their values.\n>\n> Also, point to the difficult to find git show-ref $(git replace -l).\n>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n> ---\n>  Documentation/git-replace.txt |    8 ++++++++\n>  1 files changed, 8 insertions(+), 0 deletions(-)\n>\n> diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt\n> index 17df525..cd00837 100644\n> --- a/Documentation/git-replace.txt\n> +++ b/Documentation/git-replace.txt\n> @@ -61,6 +61,13 @@ OPTIONS\n>  \tall if no pattern is given).\n>  \tTyping \"git replace\" without arguments, also lists all replace\n>  \trefs.\n> ++\n> +Note that this lists the names of the replace refs, not their values\n> +(not their replacements). You can get the latter like this, e.g.:\n\nHmm, the update is _not wrong_ per-se, but...\n\nI highly doubt we would want to try to cover confusions that may come from\nany and all different mis-/pre-conceptions people may have by making the\ndescription _longer_.\n\nWhich part of the wording in the existing description made you think that\nthe command might list both names and their contents?  We should identify\nthat misleading description (if there is) and fix that, instead of tacking\nclarifying clauses at the end.\n\nGiven these statements:\n\n\t\"git replace\" lists all replace refs.\n        \"ls\" lists paths in the directory.\n\nI would say a natural reading of them is that they list \"replace refs\" and\n\"paths\", not \"replace refs and their contents\" and \"paths and their contents\".\n\nBy the way, one thing I forgot to say was that I do not think the variant\nof the output you wanted to have is necessarily a bad thing (it is bad to\nchange the existing output to that variant, breaking other\npeople). Perhaps it can become \"-v(erbose)\" output?\n"},{"id":"174377","messageId":"4E58FA20.4030204@drmicha.warpmail.net","threadId":"28216","inReplyTo":"7v4o14f844.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2] git-replace.txt: Clarify list mode","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-08-27T14:07:28Z","receivedAt":"2011-08-27T14:07:28Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 26.08.2011 18:30:\n> Michael J Gruber <git@drmicha.warpmail.net> writes:\n> \n>> Clarify that in list mode, \"git replace\" outputs the shortened ref\n>> names, not their values.\n>>\n>> Also, point to the difficult to find git show-ref $(git replace -l).\n>>\n>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n>> ---\n>>  Documentation/git-replace.txt |    8 ++++++++\n>>  1 files changed, 8 insertions(+), 0 deletions(-)\n>>\n>> diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt\n>> index 17df525..cd00837 100644\n>> --- a/Documentation/git-replace.txt\n>> +++ b/Documentation/git-replace.txt\n>> @@ -61,6 +61,13 @@ OPTIONS\n>>  \tall if no pattern is given).\n>>  \tTyping \"git replace\" without arguments, also lists all replace\n>>  \trefs.\n>> ++\n>> +Note that this lists the names of the replace refs, not their values\n>> +(not their replacements). You can get the latter like this, e.g.:\n> \n> Hmm, the update is _not wrong_ per-se, but...\n> \n> I highly doubt we would want to try to cover confusions that may come from\n> any and all different mis-/pre-conceptions people may have by making the\n> description _longer_.\n> \n> Which part of the wording in the existing description made you think that\n> the command might list both names and their contents?  We should identify\n> that misleading description (if there is) and fix that, instead of tacking\n> clarifying clauses at the end.\n\nFull disclosure: I misunderstood it when I read it the first time. I\nnever expected it to list pairs of refs, but the questions is: Does it\nlist the original sha1 or the replacement sha1?\n\n\"replace ref\" can be very easily misunderstood as \"the ref which\nreplaces\", i.e. the replacement sha1. I know a ref is not a rev, but\n\"replace ref\" can easily be misread as \"replace rev\", \"replaced ref\" etc.\n\nSecondly, \"git replace -l sha1\" is completely useless, and I did not\nexpect that either. Granted, it outputs sha1 or not, depending on\nwhether it is replaced or not, so \"completely\" is a bit harsh, but still.\n\nSo, while I still have things to learn about git, I've also had my share\nof exposure to refs and revs, and if I misunderstand a man page, it\nindicates that there may be others whom I could help with what I've\nlearned from my own misunderstandings.\n\nAlso, as regards to clarifying: In out man pages, we often show\npractical, non-obvious ways in which a user can combine commands, and I\nthink git show-ref $(git replace -l) is one of them.\n\n> Given these statements:\n> \n> \t\"git replace\" lists all replace refs.\n>         \"ls\" lists paths in the directory.\n> \n> I would say a natural reading of them is that they list \"replace refs\" and\n> \"paths\", not \"replace refs and their contents\" and \"paths and their contents\".\n\nThis is natural, and the confusion above is a non-issue, *if* the reader\nis very aware of the implementation of replacements as lightweight\ntag-like refs with the sha1 as refname.\n\n> By the way, one thing I forgot to say was that I do not think the variant\n> of the output you wanted to have is necessarily a bad thing (it is bad to\n> change the existing output to that variant, breaking other\n> people). Perhaps it can become \"-v(erbose)\" output?\n\nI'd say yes, Christian seems to prefer having \"-v\" even closer to\n\"branch -v\". I'd say a user has the right to expect \"-v,--verbose\"\ngiving more information, but the type and extent of information should\ndepend on the actual command :)\n\nMichael\n"}]}