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

Add "enablePatterns" config option #92

Merged
merged 1 commit into from
Aug 5, 2020

Conversation

Flaque
Copy link
Contributor

@Flaque Flaque commented Jun 19, 2020

Overview

This PR adds a deno.enablePatterns config option that lets you specify particular paths or folders where the extension is enabled. This is useful for projects that contain both Node code and Deno code, such as the vscode_deno repo itself. This relates to #87

Example usage:

{
  "deno.enablePatterns": ["packages/deno/**", "*.deno.ts"]
}

Each item in the array matches a regex and only one item needs to match to enable the extension on that file.

Disclaimer

This is more of a suggestion. Totally understand if this is the wrong naming convention, or the wrong way to go about this.

Testing Done

I've played around with this in a number of different configurations in this example repo I setup. Though it might be a good idea for someone else to play around with it.

@jsejcksn
Copy link
Contributor

jsejcksn commented Jun 30, 2020

Some questions to consider:

  • Is the intended use for this to match patterns relative to the workspace directory or root directory?
  • Regex patterns and/or glob patterns?
  • Is deno.enablePatterns a more suitable name for an array of patterns? (ref. prior art: ESLint, Jest)

@Flaque
Copy link
Contributor Author

Flaque commented Jul 4, 2020

@jsejcksn

Is the intended use for this to match patterns relative to the workspace directory or root directory?

Workspace directory, I'll add a note in the docs.

Regex patterns and/or glob patterns?

Regex, and same.

enablePatterns

Agreed, that's a better name, I'll update.

@Flaque Flaque force-pushed the paths branch 2 times, most recently from 4ba6e8d to 6e5e363 Compare July 4, 2020 22:39
@Flaque Flaque changed the title Add enabled "paths" config option Add enabled "enablePaths" config option Jul 4, 2020
@Flaque Flaque changed the title Add enabled "enablePaths" config option Add enabled "enablePatterns" config option Jul 4, 2020
@Flaque Flaque changed the title Add enabled "enablePatterns" config option Add "enablePatterns" config option Jul 4, 2020
@David-Else
Copy link

@Flaque This looks amazing! I am trying to use Deno just for testing along with Snowpack for building. I need the test directory to be just for Deno test code with it's .ts extensions. The latest update on that seems to be microsoft/TypeScript#37582

Who has the authority to merge this, it has been nearly a month. Do you think it is ready now?

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, sorry it took so long @Flaque, thanks for this contribution.

@bartlomieju bartlomieju merged commit ed4aebb into denoland:master Aug 5, 2020
@Flaque
Copy link
Contributor Author

Flaque commented Aug 5, 2020

🎉🎉🎉🎉

turadg added a commit to turadg/vscode_deno that referenced this pull request Aug 10, 2020
A new config option, `deno.enablePatterns`, was added in denoland#92. The [config schema](https://code.visualstudio.com/api/references/contribution-points#contributes.configuration) wasn't added, probably because there's no UI editor for "array" type. However, it's still worth defining.

1. To avoid this misleading feedback:

2. To get an indication in the Settings UI that the key exists and can be edited in the JSON:
> Other types, such as object and array, aren't exposed directly in the settings UI, and can only be modified by editing the JSON directly. Instead of controls for editing them, users will see a link to Edit in settings.json as shown in the screenshot above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants