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

Update launch.json #8837

Open
wants to merge 2 commits into
base: svelte-4
Choose a base branch
from
Open

Conversation

KARTHIK07GUPTA
Copy link

Improvements Made:

Added the "webRoot" property to the browser configuration to specify the web root directory as the workspace folder. Modified the "program" property in the server configuration to use an absolute path for the start.js script. Added the "restart" property to the server configuration to enable automatic restarts. Set the "console" property to "integratedTerminal" for the server configuration to use an integrated terminal for debugging. These improvements aim to enhance the functionality and usability of the launch configurations. Adjust the paths and properties according to your project's structure and requirements.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Improvements Made:

Added the "webRoot" property to the browser configuration to specify the web root directory as the workspace folder.
Modified the "program" property in the server configuration to use an absolute path for the start.js script.
Added the "restart" property to the server configuration to enable automatic restarts.
Set the "console" property to "integratedTerminal" for the server configuration to use an integrated terminal for debugging.
These improvements aim to enhance the functionality and usability of the launch configurations. Adjust the paths and properties according to your project's structure and requirements.
@Conduitry
Copy link
Member

Can you change the indentation back to tabs please? It's difficult to tell what this changes when almost every line has changed.

@@ -5,17 +5,19 @@
"type": "chrome",
"request": "launch",
"name": "Playground: Browser",
"url": "http://localhost:10001"
"url": "http://localhost:10001",
"webRoot": "${workspaceFolder}"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't exactly know what this option is used for but from its description it sounds like it should point to the directory that the files are served from and pointing it to the workspace folder would be incorrect. Could you please explain what this does?

},
{
"type": "node",
"request": "launch",
"runtimeArgs": ["--watch"],
"name": "Playground: Server",
"outputCapture": "std",
"program": "start.js",
"program": "${workspaceFolder}/packages/playground/start.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not necessary, it resolves using the cwd below.

"cwd": "${workspaceFolder}/packages/playground",
"cascadeTerminateToConfigurations": ["Playground: Browser"]
"restart": true,
"console": "integratedTerminal"
Copy link
Contributor

Choose a reason for hiding this comment

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

This will make the output appear in the terminal instead of the debug console, is there any benefit to doing so?

@benmccann
Copy link
Member

@KARTHIK07GUPTA can you explain why each change was made? I.e. please explain what the behavior is before and after and why you prefer the new behavior. If there's no response we'll go ahead and close this PR

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.

None yet

5 participants