Fix keywords - Githubissues

nwolverson / vscode-language-purescript

PureScript Language Support for Visual Studio Code

https://marketplace.visualstudio.com/items/nwolverson.language-purescript

Other

17 stars 12 forks source link

Fix keywords #19

Closed wclr closed 3 years ago

wclr commented 3 years ago

@nwolverson

Added updates of purescript.json from the published version to vscode store, in git it seems to be not actual version (0.2.3) while in the store it is (0.2.4), but I didn't update package.json, so package.json needs to be updated.
Added keyword syntax fixes, most of the keywords like module, data, type are legal only at the beginning of the line and can be used for example as record field names. Example of changes:

Added Test file Main.purs where changes could be visually assessed. Though it doesn't contain legal syntax (I just turned off language-server for this workspace). So maybe it is worth making it syntactically correct, but it would require adding some more files to test different modules declarations.

If you ok with those changes, I probably need to do this (make it syntactically correct) and add more syntax cases to assess.

nwolverson commented 3 years ago

Hi, please read the README.md and CONTRIBUTING.md, I can't accept any PR to the grammar in this repository as it is generated as described from the purescript.coffee in https://github.com/nwolverson/atom-language-purescript - which is also where GitHub syntax highlighting comes from.

I understand that's not the easiest contribution method for a lot of people - so for sure it's useful to have a description of issues observed and suggestions/experiments of how to fix, but issues should probably be raised on the atom repo, and in particular any discussion of tradeoffs etc should probably be there.

wclr commented 3 years ago

@nwolverson ah, I was inattentive, ok I thought this is an upstream repo, so I need to make changes in the coffee script source and commit to https://github.com/nwolverson/atom-language-purescript ?

wclr commented 3 years ago

@nwolverson but what about the test file for assessment, shouldn't it go to this repo?

Also, why not merge the repos into a single one? The management would be kind of easier I believe?

nwolverson commented 3 years ago

I don't object to a syntax-highlight test file, we can upstream that too. The grammar in this repo is pulled down in a shell script, there is little management overhead.

I don't want to merge the repositories for an atom package and a vscode extension, I don't think it makes any sense, would require effort to set up (and I'm not sure if there are any blockers).

Again if it's a problem to you to contribute, I understand, I'm happy to move changes over.

wclr commented 3 years ago

Well actually it is not a problem for me to make those changes in cofee script and make another PR, but I just would like to improve the overall processes as well, to make the whole scheme simpler and further contributions easier.

I don't want to merge the repositories for an atom package and a vscode extension, I don't think it makes any sense, would require effort to set up (and I'm not sure if there are any blockers).

Maybe we could organize new repo in contrib? I think this would be easier to implement as a non-breaking change, and if there will be some blockers they will show up (without breaking anything). I could do it, seems it won't require much work.

Managing related things in one repo tends to be much simpler and easier eventually, though it may require some initial efforts to set up.

wclr commented 3 years ago

Ok, it was just a suggestion, I believe there some things to consider for changing the current state of affairs.

wclr commented 3 years ago

Will close it in favor of https://github.com/purescript-contrib/atom-language-purescript/pull/55