Skip to content

Conversation

@Ikuyadeu
Copy link
Contributor

This syntax in my REditorSupport/vscode-R#10 based on https://github.com/randy3k/R-Box.
But I think this syntax file should in this project.
Improve

  • Roxygen comments
  • Call function
  • Some buildin function

before
screen shot 2017-06-19 at 13 46 36
after
screen shot 2017-06-19 at 13 47 01

but making new test is not yet, so i'll update test_r.json as soon as possible if need.

@mention-bot
Copy link

@Ikuyadeu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @egamma to be a potential reviewer.

"version": "0.1.0",
"publisher": "vscode",
"engines": { "vscode": "*" },
"scripts": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep the script and point it to the repo that has the improved grammar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This syntax based on https://github.com/randy3k/R-Box/blob/master/syntax/R%20Extended.sublime-syntax and that is using sublime-syntax instead of tmbundle. I think "update-grammar" not support this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returned.

"grammars": [{
"language": "r",
"scopeName": "source.r",
"path": "./syntaxes/r.tmLanguage.json"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep using the .tmLanguage.json file extension. It makes it easier for us to distinguish the various JSON files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, fixed.

@aeschli
Copy link
Contributor

aeschli commented Jun 19, 2017

@Ikuyadeu How did you convert it? Manually, or is there a script? We can add that to our 'update-grammar' script

@Ikuyadeu
Copy link
Contributor Author

Ikuyadeu commented Jun 19, 2017

@aeschli First convert by script sublime-syntax(as yaml) to json, but some values(ex:push) are Manually. So script is not complete yet. I'll submit this script if completed.

@Ikuyadeu
Copy link
Contributor Author

@aeschli Thank you for your review, I fixed.

@aeschli
Copy link
Contributor

aeschli commented Jul 18, 2017

Did you had a chance to look at a script?

@Ikuyadeu
Copy link
Contributor Author

Ikuyadeu commented Sep 9, 2017

@aeschli I just used this script

var inputfile = 'R.sublime-syntax',
outputfile = 'r.json',
yaml = require('yamljs'),
fs = require('fs'),
obj = yaml.load(inputfile);

fs.writeFileSync(outputfile, JSON.stringify(obj, null, 2));

And fixed manually.

I think better way is importing converted json from here by update-grammar.
It need just change r/package.json script url.

@aeschli
Copy link
Contributor

aeschli commented Sep 11, 2017

Thanks for the pointers.
I'm curious how the sublime syntax to TextMate conversion works. I was under the impression that the difference is more than just semantics.
We need to do the conversion using a script this as we want to be able to continuously update to the latest grammar

@Ikuyadeu
Copy link
Contributor Author

Thank you share your think, actually copy sublime syntax is difficult.
But https://github.com/textmate/r.tmbundle/blob/master/Syntaxes/R.plist is not updated in 5 years.
How is change https://github.com/Microsoft/vscode/blob/master/extensions/r/package.json#L7
to

"update-grammar": "node ../../build/npm/update-grammar.js textmate/vscode-r syntax/r.json ./syntaxes/r.tmLanguage.json"

or

"update-grammar": "node ../../build/npm/update-grammar.js lee-dohm/language-r grammars/r.cson ./syntaxes/r.tmLanguage.json"

@aeschli
Copy link
Contributor

aeschli commented Sep 18, 2017

I'm happy to switch to a better grammar. Important for us would be that

  • it is maintained
  • has an OS licence statement, preferably MIT

https://github.com/lee-dohm/language-r also is no longer maintained. But if it's the best option out there, I can switch to it.

Or are you are interested in hosting and maintaining the grammar? If so, set up a repo!

@Ikuyadeu
Copy link
Contributor Author

Ikuyadeu commented Sep 18, 2017

@aeschli Yes, I'll maintain my repository until better repository is created(e.g. RTVS team will create official extension).
But I'll update grammar only when https://github.com/randy3k/R-Box is updated or issue/PR is submitted here/mine.

@aeschli
Copy link
Contributor

aeschli commented Sep 18, 2017

@Ikuyadeu Great. What's the URL of the repo?

@Ikuyadeu
Copy link
Contributor Author

@aeschli Here is my repo's syntax file https://github.com/Ikuyadeu/vscode-R/blob/master/syntax/r.json.

@aeschli
Copy link
Contributor

aeschli commented Sep 18, 2017

Thanks @Ikuyadeu. It would be nice to have a separate repo just for the grammar, to signal that also other products can use it.
But for now it's fine, I'll update to it.

@aeschli
Copy link
Contributor

aeschli commented Sep 18, 2017

I switched to your repo: 012d7da
Closing the PR, thanks @Ikuyadeu for your work!

@aeschli aeschli closed this Sep 18, 2017
@Ikuyadeu Ikuyadeu deleted the Rsupport branch September 19, 2017 00:18
@Ikuyadeu
Copy link
Contributor Author

@aeschli Thank you.

@Ikuyadeu
Copy link
Contributor Author

Ikuyadeu commented Oct 5, 2017

@aeschli Could you put about 012d7da that improved R grammer to the https://github.com/Microsoft/vscode-docs/blob/vnext/release-notes/v1_17.md#languages?
If you inform to users I can improve the syntax based on the user's issue.

@aeschli
Copy link
Contributor

aeschli commented Oct 5, 2017

@Ikuyadeu
Copy link
Contributor Author

Ikuyadeu commented Oct 6, 2017

@aeschli Thank you for your prompt work!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants