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.

Sign up for GitHub

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

Jump to bottom

code -r true.txt fails #78440

Closed
azdavis opened this issue Aug 4, 2019 · 7 comments
Closed

code -r true.txt fails #78440

azdavis opened this issue Aug 4, 2019 · 7 comments
Assignees
@joaomoreno @bpasero
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities verified Verification succeeded workbench-os-integration Native OS integration issues
Milestone
August 2019

Comments

@azdavis
Copy link
Contributor

azdavis commented Aug 4, 2019 •
edited

Issue Type: Bug

  1. open a terminal
  2. type code -r true.txt

expected: vscode is activated and the file true.txt is opened

actual: vscode is activated, but the file true.txt is not opened

VS Code version: Code 1.36.1 (2213894, 2019-07-08T22:56:38.504Z)
OS version: Darwin x64 18.7.0

System Info
Item Value
CPUs Intel(R) Core(TM) i7-5557U CPU @ 3.10GHz (4 x 3100)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: enabled
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_deferred_display_list: disabled_off
skia_renderer: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) 1, 2, 1
Memory (System) 16.00GB (2.65GB free)
Process Argv -r true.txt
Screen Reader no
VM 0%
Extensions (10)
Extension Author (truncated) Version
better-toml bun 0.3.2
transformer dak 1.6.0
prettier-vscode esb 1.9.0
sml fre 0.0.18
Go ms- 0.11.4
rust rus 0.6.1
rewrap stk 1.9.1
code-spell-checker str 1.7.17
change-case wma 1.0.0
vscode-proto3 zxh 0.3.0
@bpasero bpasero added help wanted Issues identified as good community contribution opportunities workbench-os-integration Native OS integration issues bug Issue identified by VS Code Team member as probable bug labels Aug 5, 2019
@bpasero
Copy link
Member

bpasero commented Aug 5, 2019 •
edited

@joaomoreno didn't we once have this issue where our CLI parser would choke over flags that have reserved JS words in them, as in this case "true"? I think we have this issue with numbers before if I remember correctly.

Anyhow, up for grab if someone wants to help out.

@skprabhanjan
Copy link
Contributor

@bpasero , I would like to take this up.
Some code pointers would be helpful , Thanks :)

@bpasero
Copy link
Member

bpasero commented Aug 5, 2019

@skprabhanjan thanks, a good start would be to try to reproduce running out of sources, I assume you are setup. Then I wonder why the second instance that is opening does not contain the proper environment from the command line (sends it over to the first instance here).

@skprabhanjan
Copy link
Contributor

skprabhanjan commented Aug 6, 2019 •
edited

Hi @bpasero , After debugging for long time this is what I found

  • This is where the problem is -> parsing the args here
  • According to the npm library doc of minimist , The opts.boolean should be used with double hyphen and clearly it is stated that single hyphen are not treated as boolean ( Which is the type used for reuse-window arg)
  • The same issue occurs for ( code -n true.txt which is Force to open a new window. )
  • The ideal usage should be code --r true.txt and it works perfectly ( As well as code --reuse-window true.txt)
  • code -r somethingelse.txt ( there is something happening in the call to minimist which does not return proper args when the txt file name is true )

So please go through this and get me know if this makes sense ( Not sure if we need to update the text that comes up when we type code --help or should we parse the args differently to make sure code -r works )

@bpasero
Copy link
Member

bpasero commented Aug 6, 2019

@joaomoreno thoughts on #78440 (comment) ?

@joaomoreno joaomoreno added this to the August 2019 milestone Aug 7, 2019
@joaomoreno
Copy link
Member

joaomoreno commented Aug 7, 2019 •
edited

@skprabhanjan We can't force people to use --r true.txt, which makes that solution far from ideal usage. In any case, r is a single dash boolean option so it should never be used with double dashes.

This is a bug on minimist. I've submitted a PR to fix it https://github.com/substack/minimist/pull/138, but since minimist seems dead I just published our own version of it: https://www.npmjs.com/package/vscode-minimist.

I've also added a test for this: https://github.com/microsoft/vscode/blob/56e836965104c8b76f65a76409c02cb44389e728/src/vs/platform/environment/test/node/environmentService.test.ts#L56:L65

joaomoreno added a commit that referenced this issue Aug 7, 2019
@joaomoreno
fixes #78440
56e8369
@skprabhanjan
Copy link
Contributor

@joaomoreno , That regex fix was amazing , Cheers :)

@mjbvz mjbvz added the verified Verification succeeded label Aug 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joaomoreno joaomoreno

@bpasero bpasero

Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities verified Verification succeeded workbench-os-integration Native OS integration issues
Projects
None yet
Milestone
August 2019
Development

No branches or pull requests
5 participants
@joaomoreno @bpasero @azdavis @mjbvz @skprabhanjan