The implementation is currently wrong. It breaks the @$(which ls) example from the tutorial and it is not consistent with the bash's behavior either.

Thanks for pushing on this

OK, the @$(cmd) operator's behavior is now consistent with $(cmd) in bash.
It still can't do basic ls /lib/modules/@$(uname -r)/kernel though. Parser adjustment required...

Edit: I don't really understand the failing test case.

The @$ operator does not require the leading white space character any more. Now I can do

$ echo %%%%@$(which ls)
%%%% ls --color=auto -v --group-directories-first

There is, however still an extra space between the literal argument and the output of the subprocess. @scopatz, do you have a clue about where this extra space comes from?

Maybe it is coming from https://github.com/xonsh/xonsh/blob/master/xonsh/built_ins.py#L975?

@scopatz, thanks for the suggestion, but no, the toks list does not contain the spaces.

There is something that's ensuring the subproc_arg tokens to get space-separated.

$ echo a@$(which cp)@$(which cp)
a /usr/bin/cp /usr/bin/cp

Yeah, I wonder if the lexer itself is adding this token (WS)?

Yeah, maybe it is this line: https://github.com/xonsh/xonsh/blob/master/xonsh/lexer.py#L349

So, I'm back to this PR after several months.
I still can't understand where the surrounding space characters come from. The @$ operator is translated internally by the parser to a call to __xonsh__.subproc_captured_inject which by itself does not return any extra spaces with this patchset applied. Yet, as can be seen below, the @$ operator yields both extra leading and trailing space.

$ echo _@(1)_
_1_
$ echo _@($(pidof init).rstrip())_
_1_
$ echo _@$(pidof init)_
_ 1 _
$ __xonsh__.subproc_captured_inject(["pidof", "init"])
['1']
$ echo _@(__xonsh__.subproc_captured_inject(["pidof", "init"]))_
_1_

Never mind. I know what the problem is - it's that I'm parsing the @$ operator without leading whitespace as a separate subprocess argument, which is obviously wrong. I need to concatenate the resulting expression with the previous argument instead.

This is now ready to land.
The @$() operator now behaves accordingly to the tutorial and consistently with Bash $() operator. It doesn't require leading and trailing whitespace anymore, so cd /lib/modules/@$(uname -r)/kernel and similar are now possible.

Closes #2755
Closes #3457

@laloch Any idea some test fail. Are they related to this?

@laloch Any idea some test fail. Are they related to this?

Yeah, I would like to know too, but I don't think they might be related.

Please do not merge. This is wrong once again. It breaks @() outer product expansion.

Now the @$() operator behaves like @() including the outer product expansion, so now _@$(ls)_ yields

$ cd @$(mktemp -d) and touch a b c d
$ echo _@$(ls)_
_a_ _b_ _c_ _d_

, which is not desirable, since I aim for consistency with $() operator in Bash.

Edit: maybe it is more important to be consistent internally. Maybe I should forget about Bash and keep this consistent with our @() operator instead. Any thoughts?

Thanks so much for this @laloch! Can you add a couple of cases to test_parser.py?

I'm pretty sure this is now ready to land - this time for real.

Windows on Python 3.8 keeps failing. Can you look into it please?

The failures seem all to be of similar nature - either builtins or __xonsh__ attribute doesn't exist without any previous error. That happens quite often and I don't have the slightest clue about how that can be possible.

Edit: I mean

        builtins.__xonsh__.path_literal = path_literal
>       builtins.__xonsh__.builtins = _BuiltIns(execer=execer)
E       AttributeError: module 'builtins' has no attribute '__xonsh__

Well, really?

Hey @laloch - maybe we can get these tests to pass by merging from master. Not sure if you did that just now?

Yeah, I just did a rebase on master, that was supposed to trigger the GitHub CI. It didn't.

Yeah. Weird. Maybe push an empty commit to try to trigger it?

I did some interactive rebasing, rewording and squashing - still the same. Let's try a dummy commit now...

Nope. Still the same :(

OK, I am going to try to close and reopen

Yeah, didn't work. I'll just merge once this passes

I can try to open a new PR for the same branch...

Yeah, if you want that would be nice

No problem. Let's try it.
Closing to open a new one with the same contents.