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

doc about findFiles in vscode-api #38464

Closed
wangtao0101 opened this issue Nov 16, 2017 · 21 comments
Closed

doc about findFiles in vscode-api #38464

wangtao0101 opened this issue Nov 16, 2017 · 21 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@wangtao0101
Copy link

below code come from vscode

findFiles('**∕*.js', '**∕node_modules∕**', 10)

when i copy the example code from vscode,
it can work fine before 1.18.0, but after 1.18.0, it can't work.

Actually i find that the backslash ( ∕ ) in the code is so strange, it should be ( / ).

Can you fix the mistake?

@vscodebot vscodebot bot added the api label Nov 16, 2017
@roblourens
Copy link
Member

What do you mean, it doesn't work? Please include more details.

@roblourens roblourens added search Search widget and operation issues and removed api labels Nov 16, 2017
@roblourens roblourens self-assigned this Nov 16, 2017
@wangtao0101
Copy link
Author

@roblourens I just mean the document of vscode api should not use wrong backslash ( ∕ ) should use ( / ).
Ther findFiles api can't work after 1.18.0 when using wrong backslash ( ∕ )

@roblourens
Copy link
Member

Oh I see, it is using a fancy slash character. @octref Do you know why that is? Copy/paste of that code snippet doesn't work.

@octref
Copy link
Contributor

octref commented Nov 17, 2017

@jrieken I found #891 and dd4f269

Which way should I go forward? I can introduce a build step that manually changes the magical slash to normal slash in website. But is there anyway in JSDoc to fix it?

@jrieken
Copy link
Member

jrieken commented Nov 20, 2017

But is there anyway in JSDoc to fix it?

That's the question... I have used a weird unicode character because glob-patterns in comments tend to end the comment... I could use backslashes instead of that character but I am unsure how we handle backslashes in glob-patterns...

@jrieken jrieken added api and removed search Search widget and operation issues labels Nov 20, 2017
@jrieken jrieken assigned jrieken and unassigned roblourens Nov 20, 2017
@jrieken jrieken added this to the November 2017 milestone Nov 20, 2017
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Nov 20, 2017
@roblourens
Copy link
Member

I would rather encourage people to use forward slash, which looks better and is less-likely to break cross-platform. Especially given #32533

Can we fix it on the website's end then?

@octref
Copy link
Contributor

octref commented Nov 20, 2017

I would rather encourage people to use forward slash

Agreed. Problem is in JSDoc this would break syntax-highlight (same for GitHub).

/**
 * findFiles('**/*.js', '**/node_modules/**', 10)
 */

So there are two solutions:

  1. @mjbvz Is it possible to fix this in JSDoc grammar so it's possible to use **/ in JSDoc?
  2. Go back to use the special unicode in vscode.d.ts, and I can look for it and do a global replace in website build.

@octref octref reopened this Nov 20, 2017
@roblourens
Copy link
Member

I don't think we can fix it with the grammar, if you write that then it's simply not valid TS/JS. Let's use the special character and fix it for the website.

@mjbvz
Copy link
Contributor

mjbvz commented Nov 20, 2017

Yes, this is part of the ecmascript spec. We cannot change it.

I thought you could escape this as *\/ but that doesn't work either.

@octref
Copy link
Contributor

octref commented Nov 20, 2017

@jrieken Do you mind reverting the changes then? I'll look for the special char and replace it wish slash in website build scripts.

@jrieken
Copy link
Member

jrieken commented Nov 21, 2017

I'll look for the special char and replace it wish slash in website build scripts.

Well, that will only help the website not with the doc hover and other in-tool-experiences... I still think backslash is fair. @roblourens Is there a way for you to fix escape characters?

@roblourens
Copy link
Member

To fix backslashes or special slash characters?

@jrieken
Copy link
Member

jrieken commented Nov 22, 2017

backslashes. our glob library apparently handles them already (correctly? @bpasero can I match on a name with backslash in it?, e.g **/back\slash.txt on mac and systems that allow for backslashes in file names?). So, I wonder if we can make this a search problem and make search understand those pattern?

@roblourens
Copy link
Member

I guess so. The comment will have to have a double backslash though. It's annoying that we have to work around it like this just because it's encoded in a block comment, but I can't think of a better solution.

@roblourens
Copy link
Member

roblourens commented Nov 22, 2017

I don't think other things (glob.ts?) handle backslashes cross-platform yet. Easy test, on mac, change the build/**/*.js rule to use backslashes.

image

Then the build/lib/*.js files show up in the file explorer.

image

@jrieken
Copy link
Member

jrieken commented Nov 22, 2017

Hm, create a folder with backslashes in it's name? It being filtered would be the correct behaviour which also means we treat a backslash like a name-character (not separator) on these platforms...

@bpasero
Copy link
Member

bpasero commented Nov 22, 2017

@jrieken it is not valid to have backslash in glob patterns but for the target paths that we match on it can either be backslash or slash, we normalize it.

@jrieken
Copy link
Member

jrieken commented Nov 22, 2017

@jrieken it is not valid to have backslash in glob patterns

Really, but it's valid to have backslashes in filenames. Is that the glob-definition or our implementation or it?

@bpasero
Copy link
Member

bpasero commented Nov 23, 2017

@jrieken at least the node-glob library calls out to this issue (https://github.com/isaacs/node-glob#windows) and others (https://www.tcl.tk/man/tcl/TclCmd/glob.htm) also seem to remind people to not use backslashes. I think the \ has special meaning in some glob implementations that it would escape the special characters like "*" or "?".

This is also repeated in http://man7.org/linux/man-pages/man7/glob.7.html which is probably the first glob implementation.

but it's valid to have backslashes in filenames

Yes, any mix of slash or backslash on any OS will produce the same result.

@jrieken
Copy link
Member

jrieken commented Nov 30, 2017

I now use *\u200b/ (zero width space) to break the */ sequence. Still not copy-paste-able but it looks nicer and more correct

@octref
Copy link
Contributor

octref commented Dec 11, 2017

This will be deployed to the website in our next release.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

6 participants