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

Infer via shebang #5149

Merged
merged 14 commits into from Oct 5, 2018

Conversation

Projects
None yet
5 participants
@haggholm
Contributor

haggholm commented Sep 27, 2018

Fixes #5122

If no file type can be inferred, this attempts to read the first line of a file and extract a shebang, which can be checked against a known list.

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory) There doesn’t seem to be any documentation on type inference to update.
  • I’ve read the contributing guidelines.

haggholm added some commits Sep 27, 2018

Add support for inferring language via common shebangs. Since this in…
…volves opening and reading (the first line) of the file just to detect the type, (a) do this check last and (b) cache it lazily.
Show resolved Hide resolved src/main/options.js Outdated

Petter Häggholm and others added some commits Sep 27, 2018

Show resolved Hide resolved src/language-js/index.js Outdated
Show resolved Hide resolved src/main/options.js Outdated
Per comments from @ikatyang, use `interpreters` info. Hence, necessar…
…ily, extract just the interpreter executable name from shebangs, rather than comparing shebangs as a whole.
Show resolved Hide resolved src/language-js/index.js Outdated
@evilebottnawi

This comment has been minimized.

Member

evilebottnawi commented Oct 1, 2018

Just information: we have same request in plugin-php, maybe it is good to add tests for plugin too

Show resolved Hide resolved src/main/options.js Outdated
Show resolved Hide resolved src/main/options.js Outdated
Show resolved Hide resolved tests_integration/cli/shebang/sample-bin3 Outdated
Show resolved Hide resolved src/main/options.js Outdated
Cleanup regexes and don’t trim the shebang line. Remove accidentally …
…committed PHP test. Add unknown-shebang test.
@haggholm

This comment has been minimized.

Contributor

haggholm commented Oct 3, 2018

@ikatyang I’m not a very experienced contributor in general, let alone here: I see I have a failing codecov check, but I’m not sure if it’s significant. Should I do anything else to have this merged?

@ikatyang

This comment has been minimized.

Member

ikatyang commented Oct 3, 2018

You could add some // istanbul ignore next for those untested (and also no need to test) catch branch to make codecov happy, though it's optional.

I generally wait for one day or two after I approved it to see if there's any comment from other maintainers, I'll merge it then if there's no objection.

@ikatyang ikatyang merged commit 1244729 into prettier:master Oct 5, 2018

9 of 10 checks passed

codecov/project 96.29% (-0.02%) compared to 3e05f21
Details
ci/circleci: build_prod Your tests passed on CircleCI!
Details
ci/circleci: checkout_code Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node4 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_node9 Your tests passed on CircleCI!
Details
ci/circleci: test_prod_standalone Your tests passed on CircleCI!
Details
codecov/patch 90.47% of diff hit (target 80%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@ikatyang

This comment has been minimized.

Member

ikatyang commented Oct 5, 2018

Thanks!

@azz

This comment has been minimized.

Member

azz commented Oct 7, 2018

Awesome! Will this work with #!node? (I see that kind of shebang every now and again)

@haggholm

This comment has been minimized.

Contributor

haggholm commented Oct 7, 2018

Awesome! Will this work with #!node? (I see that kind of shebang every now and again)

Not the way I wrote it in this PR—I’ve never seen that kind of shebang, so I didn’t know to look for it. But it should be trivial to add that.

@ikatyang ikatyang added this to the 1.15 milestone Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment