Allow using workspaceFolder placeholder variable for more portable sandbox configurations #424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rgrinberg
reviewed
Nov 2, 2020
mnxn
force-pushed
the
sandbox-path-variable
branch
from
5d83d93 to
0dcafde
Compare
rgrinberg
approved these changes
Nov 2, 2020
rgrinberg
reviewed
Nov 2, 2020
|
|
||
| type replacer = string -> string list -> string | ||
|
|
||
| val replace : string -> regexp:t -> replacer:replacer -> string |
There was a problem hiding this comment.
Surprised that this binding doesn't exist in jsoo. You should consider contributing it.
There was a problem hiding this comment.
This function is difficult to type correctly. It includes a variable number of string parameters with different parameter types after that: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Specifying_a_function_as_a_parameter
Now that I've revisited it, I think this binding is unsafe due to the trailing parameters that were omitted.
I'll try to contribute it to Js_of_ocaml if I can get a gen_js_api version working first.
mnxn
force-pushed
the
sandbox-path-variable
branch
from
c708ec7 to
515c3dc
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #374
Allows using
${workspaceFolder}for the sandbox setting's manifest/switch/template. Since this only replaces the variable, it's done during the JSON serialization/deserialization. Relative paths are still not supported, so no command execution logic was changed.In a workspace editor, it resolves
${workspaceFolder}with the path of the first workspace folder. If no workspace folders are open,${workspaceFolder}is resolved with the value ofProcess.cwd.When selecting a sandbox, if the path of the workspace folder appears in a discovered configuration, it will be substituted with
${workspaceFolder}. This makes the sandbox configurations portable by default.