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

json: Add runnable for package.json and composer.json scripts #12285

Merged

Conversation

RemcoSmitsDev
Copy link
Contributor

@RemcoSmitsDev RemcoSmitsDev commented May 25, 2024

Package.json

Screen.Recording.2024-05-25.at.17.42.48.mov

Composer.json

Screen.Recording.2024-05-26.at.12.27.09.mov

Release Notes:

  • Added runnable for package.json and composer.json scripts (#12215).

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label May 25, 2024
@znycheporuk
Copy link
Contributor

To change runner from npm to yarn, for example. Do I have to copy task and replace npm run with yarn, or is it possible, to configure runner somehow?

@Angelk90
Copy link

@RemcoSmitsDev , @znycheporuk : The question is whether I need to use npx or even bun to execute a command.

Before if you would open the task modal you would see a task for `scripts` which is incorrect, because you cannot run it. So changing it to a custom variable fixes this.
@RemcoSmitsDev
Copy link
Contributor Author

@znycheporuk @Angelk90

See the following comment on my JavaScript runnable PR #12118 (comment). We want to make something like that, but I'm not sure if we want to merge this and first just let people duplicate the task if they want to use a different runner or make it more user-friendly first.

@RemcoSmitsDev
Copy link
Contributor Author

@osiewicz It might be cool to be able to specify a filename pattern that the task is allowed to show up. This way, we could separate package.json and composer.json scripts and not show the runnable when your JSON file contains these keys.

This allows us to use the same query for `package.json` and `composer.json`
@RemcoSmitsDev RemcoSmitsDev changed the title json: Add runnable for package.json scripts json: Add runnable for package.json and composer.json scripts May 26, 2024
@znycheporuk
Copy link
Contributor

@RemcoSmitsDev, I agree that it would be cool to specify env variables for tasks like this

tasks:
  javascript:
    TEST_RUNNER: jest

In my opinion if adding this functionality will take an additional week or two, it's probably better to wait until it can be implemented in a more general way. However, if there's no clear timeline for when it will be possible to use such variables in tasks, then merge the PR as it is and update the task later.

@osiewicz
Copy link
Contributor

I'm out in the latter half of the next week, so I'd rather have us start with something (even if it's not ideal) and then refine it once user-provided task variables do land.

@osiewicz
Copy link
Contributor

@osiewicz It might be cool to be able to specify a filename pattern that the task is allowed to show up. This way, we could separate package.json and composer.json scripts and not show the runnable when your JSON file contains these keys.

I think that would be feasible to do, yeah

Copy link
Contributor

@osiewicz osiewicz left a comment

Choose a reason for hiding this comment

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

LGTM :)

@RemcoSmitsDev
Copy link
Contributor Author

Should be good to go now!

@osiewicz osiewicz merged commit 5665cad into zed-industries:main May 26, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants