Re: [PATCH v3 6/7] rebase: write better reflog messages
Junio C Hamano wrote: > So you are forcing people to maintain _two_ variables, instead of > just _one_, without making anything simpler. > > What's so hard to understand why it is a wrong design? Fine. Let's say I buy your argument about one-variable versus two-variables: how do you solve the existing problems that are solved by overriding GIT_REFLOG_ACTION that I pointed out? > If you truly think this is "inconsequential", that unfortunately > convinces me that you cannot yet be trusted enough to give you > latitude to design interfaces that span multiple programs X-<. *shrug* I certainly don't think one-variable versus two-variables warrants this much discussion. I don't have anything to win or lose: I designed a solution to the problem which I think is reasonable; you don't. Fine. Show me an alternative that doesn't involve rewriting half of the rebase infrastructure in a series that fixes checkout-dash. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Ramkumar Ramachandra writes: >>> + # always reset GIT_REFLOG_ACTION before calling any external >>> + # scripts; they have no idea about our base_reflog_action >>> + GIT_REFLOG_ACTION="$base_reflog_action" >>> git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" >> >> Why does this reroll still use this base_reflog_action convention? > > Because it's simple and it makes sense. Without $base_reflog_action hack, you have to make sure GIT_REFLOG_ACTION is reasonably pristine when you call out other people. Even with $base_reflog_action, you still have to do the same "keep GIT_REFLOG_ACTION pristine" like this one. And in addition, you have to maintain $base_reflog_action as if it is a read-only variable [*1*]. So you are forcing people to maintain _two_ variables, instead of just _one_, without making anything simpler. What's so hard to understand why it is a wrong design? > ... and we're discussing absolutely trivial inconsequential rubbish > once again. In any case, I've given up on arguing with you as it is > clear that I can't possibly win. Do whatever you want. It is not about winning or losing. If you truly think this is "inconsequential", that unfortunately convinces me that you cannot yet be trusted enough to give you latitude to design interfaces that span multiple programs X-<. [Footnote] *1* The original orig_reflog_action you borrowed from was bad enough but it had an excuse that it was confined within the leaf level of the callchain. It was merely done as a way to stash the vanilla action name (e.g. "rebase -i" before it is specialized into "rebase -i pick" etc) away, so that it can easily "lose" the speciailzation from GIT_REFLOG_ACTION while preparing for the next operation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Junio C Hamano wrote: >> @@ -59,6 +63,9 @@ else >> return $? >> fi >> >> + # always reset GIT_REFLOG_ACTION before calling any external >> + # scripts; they have no idea about our base_reflog_action >> + GIT_REFLOG_ACTION="$base_reflog_action" >> git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" > > Why does this reroll still use this base_reflog_action convention? Because it's simple and it makes sense. > The original orig_reflog_action you borrowed this may have been an > acceptable local solution inside > git-rebase--interactive that does not call out to amyting, but > the above comment a good demonstration that shows why this cannot be > a good general solution that scales across scriptlets. Nonsense. How do I set a custom reflog action when a certain flag is passed to my script (like git-rebase.sh:333) without *overriding* an existing GIT_REFLOG_ACTION? How do I construct cute start/pick/reword prefixes elegantly (like in git-rebase--interactive.sh:comment_for_reflog()) without *overriding* an existing GIT_REFLOG_ACTION? In both these examples, I'm setting a GIT_REFLOG_ACTION for the "rest of the code" to use. I don't care about the exact command sequence, but I know that they respect GIT_REFLOG_ACTION; so I'm setting one in advance. When calling out to an external scriptlet, I want to define my own reflog message: when I call out to "am" from "rebase -i", it should write a "rebase -i: " message, and ignoring its own set_reflog_message(). _That_ can be done using the subshell thing you proposed. And I have absolutely no clue why ( export GIT_REFLOG_ACTION git am ) is "more scalable" than GIT_REFLOG_ACTION="$base_reflog_action" git am > And I already explained that to you at least twice. You just gave set_reflog_action() and GIT_REFLOG_ACTION some sort of God status, and proposed to make the scripts more ugly and less extensible. ... and we're discussing absolutely trivial inconsequential rubbish once again. In any case, I've given up on arguing with you as it is clear that I can't possibly win. Do whatever you want. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/7] rebase: write better reflog messages
Ramkumar Ramachandra writes: > @@ -59,6 +63,9 @@ else > return $? > fi > > + # always reset GIT_REFLOG_ACTION before calling any external > + # scripts; they have no idea about our base_reflog_action > + GIT_REFLOG_ACTION="$base_reflog_action" > git am $git_am_opt --rebasing --resolvemsg="$resolvemsg" Why does this reroll still use this base_reflog_action convention? The original orig_reflog_action you borrowed this may have been an acceptable local solution inside git-rebase--interactive that does not call out to amyting, but the above comment a good demonstration that shows why this cannot be a good general solution that scales across scriptlets. And I already explained that to you at least twice. cf. http://article.gmane.org/gmane.comp.version-control.git/228399 But after writing it down this way, I realize that introduction of base_reflog_action (or GIT_REFLOG_NAME which is a moral equivalent) is not helping us at all. As long as calls to "git" command in the second category exists in these scripts, GIT_REFLOG_ACTION *must* be kept pristine after set_reflog_action sets it, so we can get rid of this new variable, and rewrite 3.a and 3.b like so: 3-a) GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message" \ git cmd 3-b) SAVED=$GIT_REFLOG_ACTION GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message" output git cmd GIT_REFLOG_ACTION=$SAVED or ( GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message" output git cmd ) That essentially boils down to the very original suggestion I made before Ram introduced the base_reflog_action. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
