Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Duy Nguyenwrites: > I don't object the alias.. approach though. It's > definitely a cleaner one in my opinion. It just needs people who can > spend time to follow up until the end. But if someone decides to do > that now, I'll drop the "(properties)!command" and try to support > him/her. I don't object to either approach, but what I would love to see people avoid is to end up with both. Thanks.
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Tue, Oct 11, 2016 at 10:01 PM, Jeff Kingwrote: > On Tue, Oct 11, 2016 at 11:44:50AM +0200, Johannes Schindelin wrote: > >> > Yeah, that's reasonable, too. So: >> > >> > [alias] >> > d2u = "!dos2unix" >> > >> > acts exactly as if: >> > >> > [alias "d2u"] >> > command = dos2unix >> > type = shell >> > >> > was specified at that point, which is easy to understand. >> >> It is easy to understand, and even easier to get wrong or out of sync. I >> really liked the ease of *one* `git config` call to add new aliases. Not >> sure that I like the need for more such calls just to add *one* alias (one >> config call for "shell", one for "don't cd up", etc). > > Could we simply support alias.d2u indefinitely, and you could use > whichever format you felt like (the shorter, more limited one if you > wanted, or the more verbose but flexible one)? Before this thread goes completely dead... Since there's a lot more work involved with the new alias.. approach (short term would be git completion support, longer term would be the ability to manipulate a config group more conveniently), I'm going with the "(properties)!command" approach. But even then a new series is not going to pop up, like, in the next two months. I don't object the alias.. approach though. It's definitely a cleaner one in my opinion. It just needs people who can spend time to follow up until the end. But if someone decides to do that now, I'll drop the "(properties)!command" and try to support him/her. -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Tue, Oct 11, 2016 at 11:44:50AM +0200, Johannes Schindelin wrote: > > Yeah, that's reasonable, too. So: > > > > [alias] > > d2u = "!dos2unix" > > > > acts exactly as if: > > > > [alias "d2u"] > > command = dos2unix > > type = shell > > > > was specified at that point, which is easy to understand. > > It is easy to understand, and even easier to get wrong or out of sync. I > really liked the ease of *one* `git config` call to add new aliases. Not > sure that I like the need for more such calls just to add *one* alias (one > config call for "shell", one for "don't cd up", etc). Could we simply support alias.d2u indefinitely, and you could use whichever format you felt like (the shorter, more limited one if you wanted, or the more verbose but flexible one)? -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
W dniu 11.10.2016 o 13:51, SZEDER Gábor pisze: > Quoting Duy Nguyen: >> On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski wrote: >>> W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: On Fri, 7 Oct 2016, Jakub Narębski wrote: >>> > Note that we would have to teach git completion about new syntax; > or new configuration variable if we go that route. Why would we? Git's completion does not expand aliases, it only completes the aliases' names, not their values. >>> >>> Because Git completion finds out which _options_ and _arguments_ >>> to complete for an alias based on its expansion. >>> >>> Yes, this is nice bit of trickery... >> >> It's c63437c (bash: improve aliased command recognition - 2010-02-23) >> isn't it? This may favor j6t's approach [1] because retrieving alias >> attributes is much easier. >> >> [1] >> https://public-inbox.org/git/20161006190720.4ecf3ptl6mszt...@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4 >> -- >> Duy > > The completion script is concerned in three ways: > > 1. it has to get the names of all aliases, to offer them along with > git commands for 'git ' or 'git help ', > > 2. it has to get the command executed in place of the alias, and > > 3. strip everything that can't be a git command, so it can offer the > right options for the aliased command. There is also a possibility to tell the completion script which git command to use for option completion via some trickery, even if the first git command of many used in script is not right (e.g. when "$@" is passed somewhere in the middle). I don't remember exact details, but let's look at source: # If you use complex aliases of form '!f() { ... }; f', you can use the null # command ':' as the first command in the function body to declare the desired # completion style. For example '!f() { : git commit ; ... }; f' will # tell the completion to use commit completion. This also works with aliases # of form "!sh -c '...'". For example, "!sh -c ': git commit ; ... '". Very nice. > > The '!!' syntax is the easiest, I think it will just work even with > the current completion code, no changes necessary. > > The '(nocd)' form is still easy, we only have to add this '(nocd)' to > that list of stripped words for (3), but no further changes necessary > for (1) and (2). Shouldn't the '!' vs '!!' / '(nocd)!' affect pathname completion? Or do tab completion in subdir offer wrong completion of filenames for aliases? Best, -- Jakub Narębski
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Quoting Duy Nguyen: On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębski wrote: W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: On Fri, 7 Oct 2016, Jakub Narębski wrote: Note that we would have to teach git completion about new syntax; or new configuration variable if we go that route. Why would we? Git's completion does not expand aliases, it only completes the aliases' names, not their values. Because Git completion finds out which _options_ and _arguments_ to complete for an alias based on its expansion. Yes, this is nice bit of trickery... It's c63437c (bash: improve aliased command recognition - 2010-02-23) isn't it? This may favor j6t's approach [1] because retrieving alias attributes is much easier. [1] https://public-inbox.org/git/20161006190720.4ecf3ptl6mszt...@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4 -- Duy The completion script is concerned in three ways: 1. it has to get the names of all aliases, to offer them along with git commands for 'git ' or 'git help ', 2. it has to get the command executed in place of the alias, and 3. strip everything that can't be a git command, so it can offer the right options for the aliased command. The '!!' syntax is the easiest, I think it will just work even with the current completion code, no changes necessary. The '(nocd)' form is still easy, we only have to add this '(nocd)' to that list of stripped words for (3), but no further changes necessary for (1) and (2). 'alias.d2u.command' is tricky. Both (1) and (2) require adjustments, preferably in a way that doesn't involve additional git processes, at least for (1), as it is executed often, for every 'git ', for the sake of users on platforms with not particularly stellar fork()+exec() performance. I think it would be possible to have only one 'git config --get-regexp' and a little bit of post processing in each case, but I didn't think too hard about possible pitfalls, especially when dealing with ambiguity when both 'alias.d2u' and 'alias.d2u.command' are set. And I won't think any more about it until a conclusion is reached that we'll go this route :)
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Duy, On Tue, 11 Oct 2016, Duy Nguyen wrote: > On Tue, Oct 11, 2016 at 4:44 PM, Johannes Schindelin >wrote: > > > > On Sun, 9 Oct 2016, Jeff King wrote: > > > >> On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote: > >> > >> > > If you mean ambiguity between the old "alias.X" and the new > >> > > "alias.X.*", > >> > > then yes, I think that's an unavoidable part of the transition. IMHO, > >> > > the new should take precedence over the old, and people will gradually > >> > > move from one to the other. > >> > > >> > Do we really need to treat this differently than > >> > > >> > [alias] > >> > d2u = !dos2unix > >> > d2u = C:/cygwin/bin/dos3unix.exe > >> > > >> > ? > >> > > >> > Another similar case is one d2u (could be either old syntax or new) is > >> > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In > >> > either case, the "latest" d2u definition wins. > >> > >> Yeah, that's reasonable, too. So: > >> > >> [alias] > >> d2u = "!dos2unix" > >> > >> acts exactly as if: > >> > >> [alias "d2u"] > >> command = dos2unix > >> type = shell > >> > >> was specified at that point, which is easy to understand. > > > > It is easy to understand, and even easier to get wrong or out of sync. I > > really liked the ease of *one* `git config` call to add new aliases. > > I was about to bring this up. Although to me, "git config --global > alias.foo bar" is more convenient, but not using it is not exactly > easy to get wrong or out of sync. For adding alias.$name.* I was > thinking about "git config --global --edit", not executing "git > config" multiple times. Right, but many of my aliases get set by scripts, so your `--edit` idea won't work for me. > > Not sure that I like the need for more such calls just to add *one* alias > > (one > > config call for "shell", one for "don't cd up", etc). > > We could add git-alias if more alias types pop up (and in my opinion > git-alias is the right call, we've been abusing git-config for alias > manipulation for a long time). Maybe. It is also possible that this issue is a good indicator that we are complicating things [*1*] more than necessary... Ciao, Dscho Footnote *1*: http://thedailywtf.com/articles/The_Complicator_0x27_s_Gloves
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Tue, Oct 11, 2016 at 4:44 PM, Johannes Schindelinwrote: > Hi, > > On Sun, 9 Oct 2016, Jeff King wrote: > >> On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote: >> >> > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*", >> > > then yes, I think that's an unavoidable part of the transition. IMHO, >> > > the new should take precedence over the old, and people will gradually >> > > move from one to the other. >> > >> > Do we really need to treat this differently than >> > >> > [alias] >> > d2u = !dos2unix >> > d2u = C:/cygwin/bin/dos3unix.exe >> > >> > ? >> > >> > Another similar case is one d2u (could be either old syntax or new) is >> > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In >> > either case, the "latest" d2u definition wins. >> >> Yeah, that's reasonable, too. So: >> >> [alias] >> d2u = "!dos2unix" >> >> acts exactly as if: >> >> [alias "d2u"] >> command = dos2unix >> type = shell >> >> was specified at that point, which is easy to understand. > > It is easy to understand, and even easier to get wrong or out of sync. I > really liked the ease of *one* `git config` call to add new aliases. I was about to bring this up. Although to me, "git config --global alias.foo bar" is more convenient, but not using it is not exactly easy to get wrong or out of sync. For adding alias.$name.* I was thinking about "git config --global --edit", not executing "git config" multiple times. > Not sure that I like the need for more such calls just to add *one* alias (one > config call for "shell", one for "don't cd up", etc). We could add git-alias if more alias types pop up (and in my opinion git-alias is the right call, we've been abusing git-config for alias manipulation for a long time). -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi, On Sun, 9 Oct 2016, Jeff King wrote: > On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote: > > > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*", > > > then yes, I think that's an unavoidable part of the transition. IMHO, > > > the new should take precedence over the old, and people will gradually > > > move from one to the other. > > > > Do we really need to treat this differently than > > > > [alias] > > d2u = !dos2unix > > d2u = C:/cygwin/bin/dos3unix.exe > > > > ? > > > > Another similar case is one d2u (could be either old syntax or new) is > > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In > > either case, the "latest" d2u definition wins. > > Yeah, that's reasonable, too. So: > > [alias] > d2u = "!dos2unix" > > acts exactly as if: > > [alias "d2u"] > command = dos2unix > type = shell > > was specified at that point, which is easy to understand. It is easy to understand, and even easier to get wrong or out of sync. I really liked the ease of *one* `git config` call to add new aliases. Not sure that I like the need for more such calls just to add *one* alias (one config call for "shell", one for "don't cd up", etc). Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Jeff Kingwrites: > ... My main motive in switching to the > "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting > arbitrary content into the key (just like pager.*, as we've discussed > elsewhere). Yup, we are on the same page. It's not too grave a mistake (we said "these are command names and do not have to be able to contain random letters" and deliberately did without the usual three-level hierarchy; I think the reasoning is even more valid for pager.*), but it does imply case insensitivity not to use three-level names, and I think it is a good move.
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Mon, Oct 10, 2016 at 10:52:26AM -0700, Junio C Hamano wrote: > Jeff Kingwrites: > > > Having separate exec/shell boolean options just punts the overlap from > > the command key to those keys. If you have two mutually exclusive > > options, I think the best thing is a single option, like: > > > > type = > > > > and then it is obvious that a second appearance of "type" overrides an > > earlier one, by our usual "last one wins" convention. As opposed to: > > > > shell = true > > exec = true > > > > where you have to understand the meaning of each option to know that > > "exec" overrides "shell". > > Good. > > Duy's "do we want to chdir or stay?" would be an orthogonal axis to > "what does the command line look like?" and "how is the command line > run?" so it adds one member to the "alias..*" family of > variables, I guess. Yeah, exactly. There may be other orthogonal axes to add, too, but I do not have any in mind myself. My main motive in switching to the "alias.$cmd.key" syntax is that it fixes the ancient mistake of putting arbitrary content into the key (just like pager.*, as we've discussed elsewhere). -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Jeff Kingwrites: > Having separate exec/shell boolean options just punts the overlap from > the command key to those keys. If you have two mutually exclusive > options, I think the best thing is a single option, like: > > type = > > and then it is obvious that a second appearance of "type" overrides an > earlier one, by our usual "last one wins" convention. As opposed to: > > shell = true > exec = true > > where you have to understand the meaning of each option to know that > "exec" overrides "shell". Good. Duy's "do we want to chdir or stay?" would be an orthogonal axis to "what does the command line look like?" and "how is the command line run?" so it adds one member to the "alias..*" family of variables, I guess.
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Sun, Oct 09, 2016 at 06:32:38PM +0700, Duy Nguyen wrote: > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*", > > then yes, I think that's an unavoidable part of the transition. IMHO, > > the new should take precedence over the old, and people will gradually > > move from one to the other. > > Do we really need to treat this differently than > > [alias] > d2u = !dos2unix > d2u = C:/cygwin/bin/dos3unix.exe > > ? > > Another similar case is one d2u (could be either old syntax or new) is > defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In > either case, the "latest" d2u definition wins. Yeah, that's reasonable, too. So: [alias] d2u = "!dos2unix" acts exactly as if: [alias "d2u"] command = dos2unix type = shell was specified at that point, which is easy to understand. > > If you mean the ambiguity between alias.X.shell and alias.X.exec in your > > example, the problem is that you have keys with overlapping meanings. > > One solution is "don't do that" (so have a key like "cmd", and another > > to select "shell or git-cmd", etc). Another is to define some rule, like > > "last one wins" (so "exec" overrides "shell" in your example). > [...] > > Any suggestion? I suppose we can have _one_ key for the command. How > to execute that command (exec, shell, nocd...) are boolean options. > People can still write conflicting things. We have been nice so far, > always dying when the user specify conflicting command line options. > We could do the same here, I guess. Having separate exec/shell boolean options just punts the overlap from the command key to those keys. If you have two mutually exclusive options, I think the best thing is a single option, like: type = and then it is obvious that a second appearance of "type" overrides an earlier one, by our usual "last one wins" convention. As opposed to: shell = true exec = true where you have to understand the meaning of each option to know that "exec" overrides "shell". -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Sun, Oct 9, 2016 at 1:01 PM, Jeff Kingwrote: > On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote: > >> > > Maybe it's time to aim for >> > > >> > > git config alias.d2u.shell \ >> > >'f() { git ls-files "$@" | xargs dos2unix; }; f' >> > > git config alias.d2u.cdup false >> > > git d2u *.c # yada! >> > >> > That would be nice. It would also allow "alias.foo_bar.shell"; right now >> > "alias.foo_bar" is forbidden because of the config syntax, which does >> > not allow underscores outside of the "subsection" name. >> >> So what about this? >> >> [alias] >> d2u = !dos2unix >> [alias "d2u"] >> shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f' >> exec = C:/cygwin64/bin/dos2unix.exe >> >> You introduce all kinds of ambiguities here that did not exist before... > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*", > then yes, I think that's an unavoidable part of the transition. IMHO, > the new should take precedence over the old, and people will gradually > move from one to the other. Do we really need to treat this differently than [alias] d2u = !dos2unix d2u = C:/cygwin/bin/dos3unix.exe ? Another similar case is one d2u (could be either old syntax or new) is defined in ~/.gitconfig and the other d2u in $GIT_DIR/config. In either case, the "latest" d2u definition wins. > If you mean the ambiguity between alias.X.shell and alias.X.exec in your > example, the problem is that you have keys with overlapping meanings. > One solution is "don't do that" (so have a key like "cmd", and another > to select "shell or git-cmd", etc). Another is to define some rule, like > "last one wins" (so "exec" overrides "shell" in your example). > > I'd prefer the "don't do that" path. The config you showed is > nonsensical, and it doesn't really matter that much how we behave. But > it is better still if we have a config scheme that makes it hard to > write nonsensical things in the first place. Any suggestion? I suppose we can have _one_ key for the command. How to execute that command (exec, shell, nocd...) are boolean options. People can still write conflicting things. We have been nice so far, always dying when the user specify conflicting command line options. We could do the same here, I guess. -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Sat, Oct 08, 2016 at 10:36:13AM +0200, Johannes Schindelin wrote: > > > Maybe it's time to aim for > > > > > > git config alias.d2u.shell \ > > >'f() { git ls-files "$@" | xargs dos2unix; }; f' > > > git config alias.d2u.cdup false > > > git d2u *.c # yada! > > > > That would be nice. It would also allow "alias.foo_bar.shell"; right now > > "alias.foo_bar" is forbidden because of the config syntax, which does > > not allow underscores outside of the "subsection" name. > > So what about this? > > [alias] > d2u = !dos2unix > [alias "d2u"] > shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f' > exec = C:/cygwin64/bin/dos2unix.exe > > You introduce all kinds of ambiguities here that did not exist before... If you mean ambiguity between the old "alias.X" and the new "alias.X.*", then yes, I think that's an unavoidable part of the transition. IMHO, the new should take precedence over the old, and people will gradually move from one to the other. If you mean the ambiguity between alias.X.shell and alias.X.exec in your example, the problem is that you have keys with overlapping meanings. One solution is "don't do that" (so have a key like "cmd", and another to select "shell or git-cmd", etc). Another is to define some rule, like "last one wins" (so "exec" overrides "shell" in your example). I'd prefer the "don't do that" path. The config you showed is nonsensical, and it doesn't really matter that much how we behave. But it is better still if we have a config scheme that makes it hard to write nonsensical things in the first place. -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Sun, Oct 09, 2016 at 02:01:49AM -0400, Jeff King wrote: > > So what about this? > > > > [alias] > > d2u = !dos2unix > > [alias "d2u"] > > shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f' > > exec = C:/cygwin64/bin/dos2unix.exe > > > > You introduce all kinds of ambiguities here that did not exist before... > > If you mean ambiguity between the old "alias.X" and the new "alias.X.*", > then yes, I think that's an unavoidable part of the transition. IMHO, > the new should take precedence over the old, and people will gradually > move from one to the other. > > If you mean the ambiguity between alias.X.shell and alias.X.exec in your > example, the problem is that you have keys with overlapping meanings. > One solution is "don't do that" (so have a key like "cmd", and another > to select "shell or git-cmd", etc). Another is to define some rule, like > "last one wins" (so "exec" overrides "shell" in your example). > > I'd prefer the "don't do that" path. The config you showed is > nonsensical, and it doesn't really matter that much how we behave. But > it is better still if we have a config scheme that makes it hard to > write nonsensical things in the first place. Just to be clear on my "don't do that", I don't mean "the user should not do that", but "do not design a config scheme with keys that have overlapping meanings". -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Peff & Hannes, On Fri, 7 Oct 2016, Jeff King wrote: > On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote: > > > Maybe it's time to aim for > > > > git config alias.d2u.shell \ > >'f() { git ls-files "$@" | xargs dos2unix; }; f' > > git config alias.d2u.cdup false > > git d2u *.c # yada! > > That would be nice. It would also allow "alias.foo_bar.shell"; right now > "alias.foo_bar" is forbidden because of the config syntax, which does > not allow underscores outside of the "subsection" name. So what about this? [alias] d2u = !dos2unix [alias "d2u"] shell = 'f() { git ls-files "$@" | xargs dos2unix; }; f' exec = C:/cygwin64/bin/dos2unix.exe You introduce all kinds of ambiguities here that did not exist before... Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Fri, Oct 7, 2016 at 10:55 PM, Jakub Narębskiwrote: > W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: >> On Fri, 7 Oct 2016, Jakub Narębski wrote: > >>> Note that we would have to teach git completion about new syntax; >>> or new configuration variable if we go that route. >> >> Why would we? Git's completion does not expand aliases, it only completes >> the aliases' names, not their values. > > Because Git completion finds out which _options_ and _arguments_ > to complete for an alias based on its expansion. > > Yes, this is nice bit of trickery... It's c63437c (bash: improve aliased command recognition - 2010-02-23) isn't it? This may favor j6t's approach [1] because retrieving alias attributes is much easier. [1] https://public-inbox.org/git/20161006190720.4ecf3ptl6mszt...@sigill.intra.peff.net/T/#mb1d7b8f31d595b05105b8ea2137756761e13dbf4 -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Fri, Oct 07, 2016 at 07:42:35PM +0200, Johannes Sixt wrote: > Maybe it's time to aim for > > git config alias.d2u.shell \ >'f() { git ls-files "$@" | xargs dos2unix; }; f' > git config alias.d2u.cdup false > git d2u *.c # yada! That would be nice. It would also allow "alias.foo_bar.shell"; right now "alias.foo_bar" is forbidden because of the config syntax, which does not allow underscores outside of the "subsection" name. -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Am 07.10.2016 um 14:27 schrieb Duy Nguyen: On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelinwrote: Hi Junio, On Thu, 6 Oct 2016, Junio C Hamano wrote: Nguyễn Thái Ngọc Duy writes: Throwing something at the mailing list to see if anybody is interested. Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make handling path arguments hard because they are relative to the original cwd. We set GIT_PREFIX to work around it, but I still think it's more natural to keep cwd where it is. We have a way to do that now after 441981b (git: simplify environment save/restore logic - 2016-01-26). It's just a matter of choosing the right syntax. I'm going with '!!'. I'm not very happy with it. But I do like this type of alias. I do not know why you are not happy with the syntax, but I personally think it brilliant, both the idea and the preliminary clean-up that made this possible with a simple patch like this. I guess he is not happy with it because "!!" is quite unintuitive a construct. I know that *I* would have been puzzled by it, asking "What the heck does this do?". Yep. And I wouldn't want to set a tradition for the next alias type '!!!'. There's no good choice to represent a new alias type with a leading symbol. This just occurred to me, however, what do you think about a new config group for it? With can have something like externalAlias.* (or some other name) that lives in parallel with alias.*. Then we don't need '!' (or '!!') at all. Maybe it's time to aim for git config alias.d2u.shell \ 'f() { git ls-files "$@" | xargs dos2unix; }; f' git config alias.d2u.cdup false git d2u *.c # yada! -- Hannes
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
W dniu 07.10.2016 o 16:19, Johannes Schindelin pisze: > On Fri, 7 Oct 2016, Jakub Narębski wrote: >> Note that we would have to teach git completion about new syntax; >> or new configuration variable if we go that route. > > Why would we? Git's completion does not expand aliases, it only completes > the aliases' names, not their values. Because Git completion finds out which _options_ and _arguments_ to complete for an alias based on its expansion. Yes, this is nice bit of trickery... -- Jakub Narębski
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Johannes Schindelinwrites: > Hi Matthieu, > > On Fri, 7 Oct 2016, Matthieu Moy wrote: > >> Another possibility: !(nocd), which leaves room >> for !(keyword1,keyword2,...) if needed later. Also, it is consistent >> with the :(word) syntax of pathspecs. > > But is this backwards-compatible? Don't we execute everything that comes > after the exclamation mark as a command-line via shell, where the > parentheses mean "open a subshell"? Good point. I can imagine someone already having [alias] foo = !(cd bar && $something) && $something_else Your proposed (keyword)! is better. -- Matthieu Moy http://www-verimag.imag.fr/~moy/
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Fri, Oct 07, 2016 at 04:11:34PM +0200, Johannes Schindelin wrote: > Possibly a better idea would be to use *another* special symbol, one that > makes intuitive sense as a modifier, such as: > > [alias] > # This works as before > xyz = !pwd > # As does this > stat = -p status > # This, however, is different: > duy = (nocd)!pwd > > This is backwards compatible as "(" is not a part of any Git command, nor > of a valid alias, nor is it commonly used as part of a git-* > executable/script. > > It is also kind of a bit more intuitive, I'd wager, and it is also > extensible to future options we may want to introduce. I like this much better (like you, I am concerned about things like "!(foo)" as conflicting with the shell). And I think your "(nocd)!pwd" example is quite readable. -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Kuba, On Fri, 7 Oct 2016, Jakub Narębski wrote: > W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze: > > On Thu, 6 Oct 2016, Junio C Hamano wrote: > >> Nguyễn Thái Ngọc Duywrites: > >> > >>> Throwing something at the mailing list to see if anybody is > >>> interested. > >>> > >>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make > >>> handling path arguments hard because they are relative to the original > >>> cwd. We set GIT_PREFIX to work around it, but I still think it's more > >>> natural to keep cwd where it is. > >>> > >>> We have a way to do that now after 441981b (git: simplify environment > >>> save/restore logic - 2016-01-26). It's just a matter of choosing the > >>> right syntax. I'm going with '!!'. I'm not very happy with it. But I > >>> do like this type of alias. > >> > >> I do not know why you are not happy with the syntax, but I > >> personally think it brilliant, both the idea and the preliminary > >> clean-up that made this possible with a simple patch like this. > > > > I guess he is not happy with it because "!!" is quite unintuitive a > > construct. I know that *I* would have been puzzled by it, asking "What the > > heck does this do?". > > Well, "!" as a prefix is not intuitive either. You do not use vi, do you? :-P In vi, if you enter command mode (typing a colon) and then want to execute, say, `pwd`, you type !pwd > Perhaps "!.", because "." is current directory, and the "." command > (that is, alias to "source") doesn't make sense in git aliases. If you want to execute, say, `pwd` in the current directory, that would mean you want to write !.pwd But that already means "execute `.pwd`"... > Note that we would have to teach git completion about new syntax; > or new configuration variable if we go that route. Why would we? Git's completion does not expand aliases, it only completes the aliases' names, not their values. Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Matthieu, On Fri, 7 Oct 2016, Matthieu Moy wrote: > Duy Nguyenwrites: > > > On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin > > wrote: > >> Hi Junio, > >> > >> On Thu, 6 Oct 2016, Junio C Hamano wrote: > >> > >>> Nguyễn Thái Ngọc Duy writes: > >>> > >>> > Throwing something at the mailing list to see if anybody is > >>> > interested. > >>> > > >>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could > >>> > make > >>> > handling path arguments hard because they are relative to the > >>> > original > >>> > cwd. We set GIT_PREFIX to work around it, but I still think it's > >>> > more > >>> > natural to keep cwd where it is. > >>> > > >>> > We have a way to do that now after 441981b (git: simplify > >>> > environment > >>> > save/restore logic - 2016-01-26). It's just a matter of choosing > >>> > the > >>> > right syntax. I'm going with '!!'. I'm not very happy with it. > >>> > But I > >>> > do like this type of alias. > >>> > >>> I do not know why you are not happy with the syntax, but I > >>> personally think it brilliant, both the idea and the preliminary > >>> clean-up that made this possible with a simple patch like this. > >> > >> I guess he is not happy with it because "!!" is quite unintuitive a > >> construct. I know that *I* would have been puzzled by it, asking > >> "What the > >> heck does this do?". > > > > Yep. And I wouldn't want to set a tradition for the next alias type > > '!!!'. There's no good choice to represent a new alias type with a > > leading symbol. This just occurred to me, however, what do you think > > about a new config group for it? With can have something like > > externalAlias.* (or some other name) that lives in parallel with > > alias.*. Then we don't need '!' (or '!!') at all. > > Another possibility: !(nocd), which leaves room > for !(keyword1,keyword2,...) if needed later. Also, it is consistent > with the :(word) syntax of pathspecs. But is this backwards-compatible? Don't we execute everything that comes after the exclamation mark as a command-line via shell, where the parentheses mean "open a subshell"? Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Duy, On Fri, 7 Oct 2016, Duy Nguyen wrote: > On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin >wrote: > > > > On Thu, 6 Oct 2016, Junio C Hamano wrote: > > > >> Nguyễn Thái Ngọc Duy writes: > >> > >> > Throwing something at the mailing list to see if anybody is > >> > interested. > >> > > >> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make > >> > handling path arguments hard because they are relative to the original > >> > cwd. We set GIT_PREFIX to work around it, but I still think it's more > >> > natural to keep cwd where it is. > >> > > >> > We have a way to do that now after 441981b (git: simplify environment > >> > save/restore logic - 2016-01-26). It's just a matter of choosing the > >> > right syntax. I'm going with '!!'. I'm not very happy with it. But I > >> > do like this type of alias. > >> > >> I do not know why you are not happy with the syntax, but I > >> personally think it brilliant, both the idea and the preliminary > >> clean-up that made this possible with a simple patch like this. > > > > I guess he is not happy with it because "!!" is quite unintuitive a > > construct. I know that *I* would have been puzzled by it, asking "What the > > heck does this do?". > > Yep. And I wouldn't want to set a tradition for the next alias type > '!!!'. There's no good choice to represent a new alias type with a > leading symbol. This just occurred to me, however, what do you think > about a new config group for it? With can have something like > externalAlias.* (or some other name) that lives in parallel with > alias.*. Then we don't need '!' (or '!!') at all. But what would the precedence be? externalAlias.xyz wins over alias.xyz? And we still would need '!' support: tons of people (including myself) rely on it. Possibly a better idea would be to use *another* special symbol, one that makes intuitive sense as a modifier, such as: [alias] # This works as before xyz = !pwd # As does this stat = -p status # This, however, is different: duy = (nocd)!pwd This is backwards compatible as "(" is not a part of any Git command, nor of a valid alias, nor is it commonly used as part of a git-* executable/script. It is also kind of a bit more intuitive, I'd wager, and it is also extensible to future options we may want to introduce. Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hello, Johannes W dniu 07.10.2016 o 13:20, Johannes Schindelin pisze: > On Thu, 6 Oct 2016, Junio C Hamano wrote: >> Nguyễn Thái Ngọc Duywrites: >> >>> Throwing something at the mailing list to see if anybody is >>> interested. >>> >>> Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make >>> handling path arguments hard because they are relative to the original >>> cwd. We set GIT_PREFIX to work around it, but I still think it's more >>> natural to keep cwd where it is. >>> >>> We have a way to do that now after 441981b (git: simplify environment >>> save/restore logic - 2016-01-26). It's just a matter of choosing the >>> right syntax. I'm going with '!!'. I'm not very happy with it. But I >>> do like this type of alias. >> >> I do not know why you are not happy with the syntax, but I >> personally think it brilliant, both the idea and the preliminary >> clean-up that made this possible with a simple patch like this. > > I guess he is not happy with it because "!!" is quite unintuitive a > construct. I know that *I* would have been puzzled by it, asking "What the > heck does this do?". Well, "!" as a prefix is not intuitive either. Perhaps "!.", because "." is current directory, and the "." command (that is, alias to "source") doesn't make sense in git aliases. Note that we would have to teach git completion about new syntax; or new configuration variable if we go that route. -- Jakub Narębski
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Duy Nguyenwrites: > On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin > wrote: >> Hi Junio, >> >> On Thu, 6 Oct 2016, Junio C Hamano wrote: >> >>> Nguyễn Thái Ngọc Duy writes: >>> >>> > Throwing something at the mailing list to see if anybody is >>> > interested. >>> > >>> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could >>> > make >>> > handling path arguments hard because they are relative to the >>> > original >>> > cwd. We set GIT_PREFIX to work around it, but I still think it's >>> > more >>> > natural to keep cwd where it is. >>> > >>> > We have a way to do that now after 441981b (git: simplify >>> > environment >>> > save/restore logic - 2016-01-26). It's just a matter of choosing >>> > the >>> > right syntax. I'm going with '!!'. I'm not very happy with it. >>> > But I >>> > do like this type of alias. >>> >>> I do not know why you are not happy with the syntax, but I >>> personally think it brilliant, both the idea and the preliminary >>> clean-up that made this possible with a simple patch like this. >> >> I guess he is not happy with it because "!!" is quite unintuitive a >> construct. I know that *I* would have been puzzled by it, asking >> "What the >> heck does this do?". > > Yep. And I wouldn't want to set a tradition for the next alias type > '!!!'. There's no good choice to represent a new alias type with a > leading symbol. This just occurred to me, however, what do you think > about a new config group for it? With can have something like > externalAlias.* (or some other name) that lives in parallel with > alias.*. Then we don't need '!' (or '!!') at all. Another possibility: !(nocd), which leaves room for !(keyword1,keyword2,...) if needed later. Also, it is consistent with the :(word) syntax of pathspecs. -- Matthieu Moy http://www-verimag.imag.fr/~moy/
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Fri, Oct 7, 2016 at 7:47 PM, Matthieu Moywrote: > Duy Nguyen writes: > >> On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelin >> wrote: >>> Hi Junio, >>> >>> On Thu, 6 Oct 2016, Junio C Hamano wrote: >>> Nguyễn Thái Ngọc Duy writes: > Throwing something at the mailing list to see if anybody is > interested. > > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could > make > handling path arguments hard because they are relative to the > original > cwd. We set GIT_PREFIX to work around it, but I still think it's > more > natural to keep cwd where it is. > > We have a way to do that now after 441981b (git: simplify > environment > save/restore logic - 2016-01-26). It's just a matter of choosing > the > right syntax. I'm going with '!!'. I'm not very happy with it. > But I > do like this type of alias. I do not know why you are not happy with the syntax, but I personally think it brilliant, both the idea and the preliminary clean-up that made this possible with a simple patch like this. >>> >>> I guess he is not happy with it because "!!" is quite unintuitive a >>> construct. I know that *I* would have been puzzled by it, asking >>> "What the >>> heck does this do?". >> >> Yep. And I wouldn't want to set a tradition for the next alias type >> '!!!'. There's no good choice to represent a new alias type with a >> leading symbol. This just occurred to me, however, what do you think >> about a new config group for it? With can have something like >> externalAlias.* (or some other name) that lives in parallel with >> alias.*. Then we don't need '!' (or '!!') at all. > > Another possibility: !(nocd), which leaves room > for !(keyword1,keyword2,...) if needed later. Also, it is consistent > with the :(word) syntax of pathspecs. This seems to solve my problem nicely. -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Fri, Oct 7, 2016 at 6:20 PM, Johannes Schindelinwrote: > Hi Junio, > > On Thu, 6 Oct 2016, Junio C Hamano wrote: > >> Nguyễn Thái Ngọc Duy writes: >> >> > Throwing something at the mailing list to see if anybody is >> > interested. >> > >> > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make >> > handling path arguments hard because they are relative to the original >> > cwd. We set GIT_PREFIX to work around it, but I still think it's more >> > natural to keep cwd where it is. >> > >> > We have a way to do that now after 441981b (git: simplify environment >> > save/restore logic - 2016-01-26). It's just a matter of choosing the >> > right syntax. I'm going with '!!'. I'm not very happy with it. But I >> > do like this type of alias. >> >> I do not know why you are not happy with the syntax, but I >> personally think it brilliant, both the idea and the preliminary >> clean-up that made this possible with a simple patch like this. > > I guess he is not happy with it because "!!" is quite unintuitive a > construct. I know that *I* would have been puzzled by it, asking "What the > heck does this do?". Yep. And I wouldn't want to set a tradition for the next alias type '!!!'. There's no good choice to represent a new alias type with a leading symbol. This just occurred to me, however, what do you think about a new config group for it? With can have something like externalAlias.* (or some other name) that lives in parallel with alias.*. Then we don't need '!' (or '!!') at all. -- Duy
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Hi Junio, On Thu, 6 Oct 2016, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duywrites: > > > Throwing something at the mailing list to see if anybody is > > interested. > > > > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make > > handling path arguments hard because they are relative to the original > > cwd. We set GIT_PREFIX to work around it, but I still think it's more > > natural to keep cwd where it is. > > > > We have a way to do that now after 441981b (git: simplify environment > > save/restore logic - 2016-01-26). It's just a matter of choosing the > > right syntax. I'm going with '!!'. I'm not very happy with it. But I > > do like this type of alias. > > I do not know why you are not happy with the syntax, but I > personally think it brilliant, both the idea and the preliminary > clean-up that made this possible with a simple patch like this. I guess he is not happy with it because "!!" is quite unintuitive a construct. I know that *I* would have been puzzled by it, asking "What the heck does this do?". Ciao, Dscho
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Thu, Oct 06, 2016 at 03:00:14PM -0400, Jeff King wrote: > PS I think your "!!" syntax conflicts with something like: > > git -c alias.has-changes='!! git diff --quiet' has-changes > >I don't know if that is worth worrying about or not. I also notice >that using "!!git diff" with no space seems broken, as it seems to >skip using the shell. I wonder if that is a bug in run-command. Nevermind this last bit. It is the shell that is complaining (rightfully) about "!git"; the error message is just confusing because it mentions $0, which contains the whole script: $ git -c alias.has-changes='!!git diff --quiet' has-changes !git diff --quiet: 1: !git diff --quiet: !git: not found The "!! git diff" syntax sill has the conflict I mentioned (though note I screwed up the quotes in what I wrote above). -Peff
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
On Thu, Oct 06, 2016 at 06:41:24PM +0700, Nguyễn Thái Ngọc Duy wrote: > Throwing something at the mailing list to see if anybody is > interested. > > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make > handling path arguments hard because they are relative to the original > cwd. We set GIT_PREFIX to work around it, but I still think it's more > natural to keep cwd where it is. > > We have a way to do that now after 441981b (git: simplify environment > save/restore logic - 2016-01-26). It's just a matter of choosing the > right syntax. I'm going with '!!'. I'm not very happy with it. But I > do like this type of alias. Hmm. I wonder if any commands will be fooled by not being moved to GIT_WORK_TREE. I know that there is a bug already in this case: export HOME=$PWD for i in one two; do git init $i && (cd $i && echo "* diff=$i" >.gitattributes && git add . && git commit -m $i) && git config --global diff.$i.textconv "sed s/^/$i:/" done cd two && git --git-dir=$(pwd)/../one/.git --work-tree=$(pwd)/../one show This shows the contents of repo one using the .gitattributes from repo two. I am not sure if the bug is that when GIT_WORK_TREE is set, git-show doesn't position itself at the toplevel of that tree, or if the attributes machinery should be more careful about looking up paths in the work-tree versus the current directory. This bug is tangential to your patch (it has nothing to do with aliases, and should be fixed regardless). But I wonder if your "!!" aliases will expose this bug more frequently. -Peff PS I think your "!!" syntax conflicts with something like: git -c alias.has-changes="!! git diff --quiet' has-changes I don't know if that is worth worrying about or not. I also notice that using "!!git diff" with no space seems broken, as it seems to skip using the shell. I wonder if that is a bug in run-command.
Re: [PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Nguyễn Thái Ngọc Duywrites: > Throwing something at the mailing list to see if anybody is > interested. > > Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make > handling path arguments hard because they are relative to the original > cwd. We set GIT_PREFIX to work around it, but I still think it's more > natural to keep cwd where it is. > > We have a way to do that now after 441981b (git: simplify environment > save/restore logic - 2016-01-26). It's just a matter of choosing the > right syntax. I'm going with '!!'. I'm not very happy with it. But I > do like this type of alias. I do not know why you are not happy with the syntax, but I personally think it brilliant, both the idea and the preliminary clean-up that made this possible with a simple patch like this. > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > git.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/git.c b/git.c > index 296857a..4c1dcf4 100644 > --- a/git.c > +++ b/git.c > @@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv) > > alias_string++; > commit_pager_choice(); > + if (*alias_string == '!') { > + keep_cwd = 0; > + alias_string++; > + } > restore_env(keep_cwd); > > child.use_shell = 1;
[PATCH/RFC] git.c: support "!!" aliases that do not move cwd
Throwing something at the mailing list to see if anybody is interested. Current '!' aliases move cwd to $GIT_WORK_TREE first, which could make handling path arguments hard because they are relative to the original cwd. We set GIT_PREFIX to work around it, but I still think it's more natural to keep cwd where it is. We have a way to do that now after 441981b (git: simplify environment save/restore logic - 2016-01-26). It's just a matter of choosing the right syntax. I'm going with '!!'. I'm not very happy with it. But I do like this type of alias. Signed-off-by: Nguyễn Thái Ngọc Duy--- git.c | 4 1 file changed, 4 insertions(+) diff --git a/git.c b/git.c index 296857a..4c1dcf4 100644 --- a/git.c +++ b/git.c @@ -252,6 +252,10 @@ static int handle_alias(int *argcp, const char ***argv) alias_string++; commit_pager_choice(); + if (*alias_string == '!') { + keep_cwd = 0; + alias_string++; + } restore_env(keep_cwd); child.use_shell = 1; -- 2.8.2.524.g6ff3d78