Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Infer via shebang#5149

Merged
ikatyang merged 14 commits into
prettier:masterprettier/prettier:masterfrom
haggholm:infer-via-shebangCopy head branch name to clipboard
Oct 5, 2018
Merged

Infer via shebang#5149
ikatyang merged 14 commits into
prettier:masterprettier/prettier:masterfrom
haggholm:infer-via-shebangCopy head branch name to clipboard

Conversation

@haggholm

@haggholm haggholm commented Sep 27, 2018

Copy link
Copy Markdown
Contributor

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.

…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.
Comment thread src/main/options.js Outdated
Comment thread src/language-js/index.js Outdated
Comment thread src/main/options.js Outdated
…ily, extract just the interpreter executable name from shebangs, rather than comparing shebangs as a whole.
Comment thread src/language-js/index.js Outdated
@alexander-akait

Copy link
Copy Markdown
Member

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

Comment thread src/main/options.js Outdated
Comment thread src/main/options.js Outdated
Comment thread tests_integration/cli/shebang/sample-bin3 Outdated
Comment thread src/main/options.js Outdated
…committed PHP test. Add unknown-shebang test.
@haggholm

haggholm commented Oct 3, 2018

Copy link
Copy Markdown
Contributor Author

@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

ikatyang commented Oct 3, 2018

Copy link
Copy Markdown
Member

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
@ikatyang

ikatyang commented Oct 5, 2018

Copy link
Copy Markdown
Member

Thanks!

@azz

azz commented Oct 7, 2018

Copy link
Copy Markdown
Member

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

@haggholm

haggholm commented Oct 7, 2018

Copy link
Copy Markdown
Contributor Author

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.

@aleclarson

Copy link
Copy Markdown

Was this ever released?

Both this PR and the issue it fixes were never mentioned in the changelog. ☺️

@haggholm

Copy link
Copy Markdown
Contributor Author

@aleclarson I’m pleased to say it was merged and released; indeed I can’t see it in the changelog, but it’s mentioned in the blog: https://prettier.io/blog/2018/11/07/1.15.0.html#api-cli

And our build scripts are much Prettier.

@lock lock Bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Mar 22, 2019
@lock lock Bot locked as resolved and limited conversation to collaborators Mar 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.