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

Added PureScript language definition #2526

Merged

Conversation

sriharshachilakapati
Copy link
Contributor

Added PureScript language definition with test cases and examples.

@RunDevelopment
Copy link
Member

Thank you for making this language definition @sriharshachilakapati!

I will review it once the issue with the many generated files is resolved. This should be quite simple. My guess is that you ran npm i instead of npm ci. The output of uglify is very sensitive to the specific uglify version used.
Deleting the local node_modules folder (and reverting changes to package.json and package-lock.json, if any), and then running npm ci, followed by npm run build should fix the issue.

@sriharshachilakapati
Copy link
Contributor Author

I was using pnpm instead of npm and it didn't respect that lock file. My bad, will correct that and push soon.

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Again, thank you for the language definition @sriharshachilakapati!

I left you a few comments with suggestions and improvements. Please take a look!

components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components/prism-purescript.js Outdated Show resolved Hide resolved
components.json Outdated Show resolved Hide resolved
tests/languages/purescript/string_feature.test Outdated Show resolved Hide resolved
@sriharshachilakapati
Copy link
Contributor Author

The syntax of PureScript follows closely to that of Haskell, and hence I derived this from Haskell's one. Will reply to the comments one by one.

@RunDevelopment
Copy link
Member

I derived this from Haskell

It's ok to use another language as the base but we usually use the extend function to do so. The function basically copies a language and lets you make some changes to it. With extend, PureScript can be implemented like this:

Prism.languages.purescript = Prism.languages.extend('haskell', {
	'keyword': /\b(?:ado|case|class|data|derive|do|else|if|in|infixl|infixr|instance|let|module|newtype|of|primitive|then|type|where)\b/,
	'import_statement': {
		// The imported or hidden names are not included in this import
		// statement. This is because we want to highlight those exactly like
		// we do for the names in the program.
		pattern: /((?:\r?\n|\r|^)\s*)import\s+(?:[A-Z][\w']*)(?:\.[A-Z][\w']*)*(?:\s+as\s+(?:[A-Z][_a-zA-Z0-9']*)(?:\.[A-Z][\w']*)*)?(?:\s+hiding\b)?/m,
		lookbehind: true,
		inside: {
			'keyword': /\b(?:import|as|hiding)\b/
		}
	},
	// These are builtin variables only. Constructors are highlighted later as a constant.
	'builtin': /\b(?:when|unless|liftA1|apply|bind|discard|join|ifM|identity|whenM|unlessM|liftM1|ap|compose|otherwise|top|bottom|recip|eq|notEq|degree|div|mod|lcm|gcd|flip|const|map|void|flap|conj|disj|not|mempty|compare|min|max|comparing|clamp|between|sub|negate|append|add|zero|mul|one|show|unit|absurd)\b/,
});

The main benefits of this approach are that you have to write less code and that improvements from the extended language will trickle down (e.g. I will make a pull to fix the exponential backtracking in Haskell).

Please use the above code. It's equivalent to your current approach.

I will close all comments on patterns that are identical to Haskell's ones.

@sriharshachilakapati
Copy link
Contributor Author

@RunDevelopment I've updated the definition to use extend, but now the test cases are failing.

1) Testing language 'purescript'
       – should pass test case 'builtin_feature':
     Error: Cannot extend 'haskell' because it is not defined in Prism.languages.
      at Object.extendTest (tests/helper/checks.js:27:10)
      at Object.object.<computed> [as extend] (tests/helper/checks.js:7:10)
      at eval (eval at <anonymous> (tests/helper/prism-loader.js:66:18), <anonymous>:3:46)
      at languageFunction (tests/helper/prism-loader.js:67:57)
      at /Users/sriharshachilakapati/Projects/prism/tests/helper/prism-loader.js:69:4
      at handleId (dependencies.js:269:13)
      at loadComponentsInOrder (dependencies.js:291:4)
      at Object.load (dependencies.js:437:12)
      at Object.loadLanguages (tests/helper/prism-loader.js:56:57)
      at Object.createInstance (tests/helper/prism-loader.js:38:18)
      at Object.runTestCase (tests/helper/test-case.js:58:29)
      at Context.<anonymous> (tests/run.js:32:16)
      at processImmediate (internal/timers.js:458:21)

Where do I specify that dependency in test suite?

@RunDevelopment
Copy link
Member

My bad. You have add "require": "haskell" to PureScript in components.json (and rebuild).

Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

A rebuild is required after making any changes to components.json or any language definition.

Also, my unresolved review comments still stand.

Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
@RunDevelopment RunDevelopment merged commit ad748a0 into PrismJS:master Aug 30, 2020
@RunDevelopment
Copy link
Member

Thank you for contributing @sriharshachilakapati!

@sriharshachilakapati
Copy link
Contributor Author

@RunDevelopment Thanks for your patience in guiding me through the PR. By the way, may I know if there are any plans for a release anytime soon?

@RunDevelopment
Copy link
Member

any plans for a release anytime soon?

No. We release rather infrequently, so maybe in a few weeks?
However, all merged features are available right now on the download page.

@sriharshachilakapati
Copy link
Contributor Author

I'm asking cause I came here from another project. If released, I can directly update the package.json file there and it should work for me.

@mAAdhaTTah
Copy link
Member

@sriharshachilakapati I can't commit to anything in particular, but I will spend some time going through PRs and looking at what we should include in our next release.

@RunDevelopment We get this question a lot. We may want to consider either a regular release cycle and/or automating the release process.

@RunDevelopment
Copy link
Member

@mAAdhaTTah I was thinking about a regular release cycle too (maybe monthly?) and automating sounds good. How would we go about automating? (I always hand it off to you after making the changelog, so how involved is the process of making a new release?)

@mAAdhaTTah
Copy link
Member

It's not super involved, I think you really just need publish perms. But if we can set up a bot to do this stuff for us, it would be a lot easier.

@RunDevelopment
Copy link
Member

RunDevelopment commented Aug 31, 2020

Perms?

We could set up a bot with GH actions. They finally support manual triggers.
I'm not sure who can use these triggers tho. Would be a problem if anybody could make a new release.

@mAAdhaTTah
Copy link
Member

perms => permissions.

We could definitely have the bot only act in response to certain users (org members, probably). I'll create an issue to flesh this out a bit.

@georgefst
Copy link

georgefst commented Sep 8, 2020

I can see I've narrowly missed the boat here, but I've always felt that highlighting some arbitrary set of "built-in" functions isn't very true to the spirit of these languages, since there isn't actually anything special about them.

There was a discussion about this in the Haskell Vscode plugin, and it was decided to remove it.

quentinvernot pushed a commit to TankerHQ/prismjs that referenced this pull request Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants