Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STDIN/STDOUT redirection of debugger doesn't work anymore after 1.67 update #148887

Closed
hw0603 opened this issue May 6, 2022 · 22 comments · Fixed by #149284
Closed

STDIN/STDOUT redirection of debugger doesn't work anymore after 1.67 update #148887

hw0603 opened this issue May 6, 2022 · 22 comments · Fixed by #149284
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded

Comments

@hw0603
Copy link

hw0603 commented May 6, 2022

Does this issue occur when all extensions are disabled?: Yes

  • VS Code Version: 1.67.0
  • OS Version: Windows 10, using cmd.exe as a default shell

Steps to Reproduce:

When debugging python(I think debuggers of other languages also got effected too), I'm using following launch.json to redirect stdin to my custom input file.

        {
            "name": "Args from input.txt(Python)",
            "type": "python",
            "request": "launch",
            "program": "${file}",
            "console": "integratedTerminal",
            "cwd": "${fileDirname}",
            "args": ["<", "${workspaceFolder}\\input.txt"]
        },

This method of implementing redirection when debugging using the < character is also introduced in the official documentation of vscode.

According to the document,

This approach requires that the "<" syntax is passed through the debugger extension and ends up unmodified in the Integrated Terminal.

and it worked like charm before updating to v1.67.0.

From changelog of v1.67, I've noticed that there was a fix related to automatically add escape character(^) before special symbols such as < or |
(See 145265 Terminal on Windows uses cmd /C, this corrupts passed arguments)

When I started debugging session, vscode is now really add ^ before <. Since < character is escaped, I cannot used redirection with the method mentioned in the document anymore.

Is this the intended result? If so, how to use redirection 'correctly' after this build?
I know a lot of people redirect stdin to input.txt and redirect stdout to output.txt when debugging, which is actually very convenient.
I think that implementing redirection by passing special characters(<, >, |) as arguments of the debugger is not a very graceful solution, but that method itself should not be eliminated.

@PyaePhyoKhant
Copy link

Having the same issue with new version in WSL. It seem some characters are prefix with \. My launch.json is "args": ["<", "${fileBasenameNoExtension}_input.txt"]. But in console, the command is filename.py \< filename_input.txt, due to prefix \, my code is stuck. It was working fine yesterday, So, I think this issue is with new release.

  • VS Code Version: 1.67.0
  • OS Version: Windows 10, WSL Ubuntu20.04

@hw0603
Copy link
Author

hw0603 commented May 6, 2022

Judjung from commit log of src/vs/workbench/contrib/debug/node/terminals.ts, I'm sure this issue is related to today's release too.

@roblourens
Copy link
Member

Yeah this is #145265. Thanks for pointing out the docs, I didn't know this was recommended. It's suspicious to me that we would want to put shell-specific syntax in the args here. That isn't what "args" means to me.

I wonder whether we could add launch config parameters like "stdinFromFile" and "stdoutToFile" (and add those to the runInTerminal request) so that vscode can come up with the right syntax for the shell. I may want to revert the change until we have a better solution. Thoughts @isidorn @weinand @connor4312? Maybe candidate

@roblourens roblourens added the info-needed Issue requires more information from poster label May 6, 2022
@connor4312
Copy link
Member

connor4312 commented May 6, 2022

I'm not a fan of allowing this. We already try to escape things from those arguments, so people doing this today have only a half-working shell syntax in their arguments. It also only works if you run it in the integratedTerminal, and is of course shell-dependent too, which feels weird.

@boardkeystown
Copy link

This is super messed up. I 100% require being able to redirect > the output. This needs to be fixed.

ages: [">","out.txt"]

was 100% working on windows before this update!

My work flow is screwed rn

@boardkeystown
Copy link

@roblourens

It's suspicious to me that we would want to put shell-specific syntax in the args here

Args mean arguments as if I was writing it after typing it in console including SHELL commands.

I want to redirect output >

Why in the world it would add ^> is crazy to me.

This has to be be bug.

@boardkeystown
Copy link

@roblourens

Additional sometimes you need to chain aliases that take > redirect or pass a redirect back in <

These are shell scripts that having the debugger "args" accept <,>,| is really important to have. It was an amazing and highly convenient way to debug.

I seriously doubt this is a feature that few people would want deprecated. I seriously don't and it's killing me it broke after this update.

@hw0603
Copy link
Author

hw0603 commented May 7, 2022

@roblourens
@connor4312

Yeah I agree that passing redirection symbols through args is somewhat weird, and I understand the need for escaping shell-specific characters. That's why I've said it's not a graceful solution.
However, many people uses the redirection feature through that way now(even if the docs have been mislead the users).

It's good improvement to escape that characters in arguments since it is literally "argument", but I think there should be official solution to redirect I/O. As mentioned above, add parameters such as "stdinFromFile" and "stdoutToFile" to launch.json might be best solution.

BTW, what kind of additional information do I have to add here? I think I mentioned all the details of the sitution but I see the "needs more info" label.

@boardkeystown
Copy link

boardkeystown commented May 7, 2022

@hw0603
I've spent 3 hours trying to come up with a work around. This is insane. So much of my workflow relied on the ability the redirect stdin and stdout using <,>. It's insane that this was deprecated. I don't even get why. It doesn't have to do any crazy checking should just copying what you type in and the shell takes over.

This is a serious issue for me.
image

@boardkeystown
Copy link

"stdinFromFile" and "stdoutToFile" to launch.json might be best solution.

"args": ["<","in.txt",">","out.txt"]

If it's about "escaping shell-specific characters" then I could live with this.

"args": ["${stdin}","in.txt","${stdout}","out.txt"]

And if it's about anything else shell then give me something like this:

"shellDefine": [
["FOO","<'"],
["BAR",">'"],
]

"args": ["${FOO}","in.txt","${BAR}","out.txt"]

ChocoNaga added a commit to yeardream-high6/coding_test that referenced this issue May 7, 2022
vscode 파이썬 디버그 할 때 표준입출력을 원하는 파일로 연결되도록 설정했습니다.
그런데 어제 vscode 1.67 패치로 정상실행이 안되네요.
microsoft/vscode#148887
그래도 편하게 쓰던 기능이라 같이 올립니다.
@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues and removed info-needed Issue requires more information from poster labels May 7, 2022
@roblourens roblourens added this to the May 2022 milestone May 7, 2022
@roblourens
Copy link
Member

Sorry for the trouble. Like I said, I didn't realize that people were depending on this behavior, and I agree that it should be supported somehow, since this is a reasonable and useful workflow.

Args mean arguments as if I was writing it after typing it in console including SHELL commands.

To me, "args" means arguments that get passed to the program you are starting.

I see a couple solutions

  • Restore the behavior of args and require users to shell-escape args in their own config, which is not good, or add escapedArgs which would be shell-escaped
  • Keep the behavior of args and add shellArgs or unescapedArgs, or the more specific stdinFromFile/ stdoutToFile

Any of this will require changes to DAP. I wonder whether anyone is trying to use any shell syntax besides >/<, in other words, is there a scenario that adding stdinFromFile/ stdoutToFile wouldn't cover?

@hw0603
Copy link
Author

hw0603 commented May 7, 2022

In my opinion, the second way

  • Keep the behavior of args and add shellArgs or unescapedArgs, or the more specific stdinFromFile / stdoutToFile

is better workaround since we can prevent use of args different way from its real meaning.

@weinand
Copy link
Contributor

weinand commented May 8, 2022

@roblourens just a few comments from vacation...

  • VS Code knows nothing about args properties. Some debug extensions introduce args in their schema contribution but VS Code cannot provide any direct help with redirecting input/output.
  • VS Code supports/implements the runInTerminal DAP request which some debug extensions use to implement their (private) args support. runInTerminal does not allow direct access to shell features (e.g. input/output redirection) because no assumptions can be made about the underlying shell or whether a shell is used at all. So escaping/quoting all shell meta-characters is correct. But we might want to add additional features to the runInTerminal DAP request so that debug adapters can implement stdinFromFile/stdoutToFile property support themselves.
  • a debug extension can introduce an alternative args property that supports shell features, e.g. a commandLine property "commandLine": "arg1 arg2 < input > output". In order to support this we would have to add support to the runInTerminal DAP request to pass a full command line unchanged to the underlying shell.

@boardkeystown
Copy link

@roblourens

is there a scenario that adding stdinFromFile/ stdoutToFile wouldn't cover?

Yes. I for one will use aliases that are shell scrips to redirect their input and output to chain together programs.

@roblourens
Copy link
Member

But we might want to add additional features to the runInTerminal DAP request so that debug adapters can implement stdinFromFile/stdoutToFile property support themselves.

Yes, this is basically my thinking. But I think I should revert the change as a candidate until we figure out a better solution.

Yes. I for one will use aliases that are shell scrips to redirect their input and output to chain together programs.

@boardkeystown could you give me an example so I understand how that wouldn't be covered by this proposal?

@roblourens roblourens added the candidate Issue identified as probable candidate for fixing in the next release label May 9, 2022
@connor4312
Copy link
Member

"stdinFromFile/stdoutToFile" makes the most sense to me. I would not be in favor of a commandLine option in the runInTerminal request since, similar to the initial problem, the DA has no knowledge of the underlying shell.

@roblourens
Copy link
Member

I wonder whether we could make everyone happy in the meantime by leaving the current fix but special-casing "<" and ">" and not escaping these? You could have the > and filename in a single arg but I don't see anyone doing that. And it would make the behavior almost the same as the intended escaping behavior.

@boardkeystown
Copy link

I wonder whether we could make everyone happy in the meantime by leaving the current fix but special-casing "<" and ">" and not escaping these? You could have the > and filename in a single arg but I don't see anyone doing that. And it would make the behavior almost the same as the intended escaping behavior.

If would work with "> out.txt" instead of ">", "out.txt" I could live with that. Or what ever you mean by special-casing. I just want it to work again or have some means to do to it again.

@roblourens
Copy link
Member

roblourens commented May 11, 2022

I mean the opposite (the > in ">", "out.txt" would not be escaped, and in "> out.txt" would be escaped)

@penagos
Copy link

penagos commented May 11, 2022

@roblourens that seems like an acceptable interim fix to me. As a C++ debug extension author (vGDB), this has also broken my extension on the most recent VSCode build and I've had to suggest users defer on upgrading for now to not break existing workflows.

@roblourens
Copy link
Member

roblourens commented May 11, 2022

Thanks. I am going with this solution for the .2 build that will go out next week. Not a perfect fix - ">out.txt" as a single argument would have previously redirected output too - but people are using ">" or "<" in all the examples I have seen so far.

It will also be in Insiders tomorrow. I noticed another issue with the escaping, #149283, which will also get fixed in Insiders

@penagos
Copy link

penagos commented May 11, 2022

@roblourens thanks for the quick fix!

@connor4312 connor4312 added the verified Verification succeeded label May 13, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants