[vim/vim] add rsc syntax (PR #9097)
zainin
Add syntax file for RouterOS script.
Let me know if there's any issues to fix before merge.
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/9097
Commit Summary
File Changes
(3 files)
- A runtime/ftplugin/rsc.vim (6)
- A runtime/syntax/rsc.vim (82)
- M src/testdir/test_filetype.vim (1)
Patch Links:
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Triage notifications on the go with GitHub Mobile for iOS or Android.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -0,0 +1,6 @@ +" Vim filetype plugin file +" Language: RouterOS scripts +" Maintainer: zainin +" Last Change: 2017-07-26 + +au BufRead,BufNewFile *.rsc set filetype=rsc
This should go into filetype.vim, not into a filetype plugin.
Christian Brabandt
@chrisbra commented on this pull request.
> @@ -0,0 +1,82 @@
+" Vim syntax file
+" Language: RouterOS scripts
+" Maintainer: zainin
+" Original Author: ndbjorne @ MikroTik forums
+" Last Change: 2017-07-26
+
+" quit when a syntax file was already loaded
+if exists("b:current_syntax")
+ finish
+endif
+
+syn case ignore
+
+set iskeyword=A-Z,a-z
this needs to be :syn iskeyword
zainin
@zainin commented on this pull request.
> @@ -0,0 +1,6 @@ +" Vim filetype plugin file +" Language: RouterOS scripts +" Maintainer: zainin +" Last Change: 2017-07-26 + +au BufRead,BufNewFile *.rsc set filetype=rsc
My bad, fixed.
zainin
@zainin commented on this pull request.
> @@ -0,0 +1,82 @@
+" Vim syntax file
+" Language: RouterOS scripts
+" Maintainer: zainin
+" Original Author: ndbjorne @ MikroTik forums
+" Last Change: 2017-07-26
+
+" quit when a syntax file was already loaded
+if exists("b:current_syntax")
+ finish
+endif
+
+syn case ignore
+
+set iskeyword=A-Z,a-z
Turns out it wasn't really necessary, so I've removed this line.
zainin
@zainin commented on this pull request.
In src/testdir/test_filetype.vim:
> @@ -422,6 +422,7 @@ let s:filename_checks = {
\ 'robots': ['robots.txt'],
\ 'rpcgen': ['file.x'],
\ 'rpl': ['file.rpl'],
+ \ 'rsc': ['file.rsc'],
@dkearns , it's already here, or am I missing something?
Doug Kearns
@dkearns commented on this pull request.
In src/testdir/test_filetype.vim:
> @@ -422,6 +422,7 @@ let s:filename_checks = {
\ 'robots': ['robots.txt'],
\ 'rpcgen': ['file.x'],
\ 'rpl': ['file.rpl'],
+ \ 'rsc': ['file.rsc'],
Nope, it's there alright. I assumed it would be in the second commit and didn't even check the first.
Sorry for wasting your time.
Doug Kearns
@dkearns commented on this pull request.
> @@ -0,0 +1,82 @@
+" Vim syntax file
+" Language: RouterOS scripts
+" Maintainer: zainin
+" Original Author: ndbjorne @ MikroTik forums
+" Last Change: 2017-07-26
+
+" quit when a syntax file was already loaded
+if exists("b:current_syntax")
+ finish
+endif
+
+syn case ignore
+
+set iskeyword=A-Z,a-z
There's a few hyphenated keywords like as-value that are being missed because 'iskeyword' doesn't include -. If you don't want to use syn-iskeyword you could make them a syn-match.
Doug Kearns
Thanks for adding this filetype.
It looks like you're missing the return command and boolean true/false.
You could also consider using the #!rsc by RouterOS\n shebang line for filetype detection. That would go in runtime/scripts.vim.
zainin
@zainin commented on this pull request.
> @@ -0,0 +1,82 @@
+" Vim syntax file
+" Language: RouterOS scripts
+" Maintainer: zainin
+" Original Author: ndbjorne @ MikroTik forums
+" Last Change: 2017-07-26
+
+" quit when a syntax file was already loaded
+if exists("b:current_syntax")
+ finish
+endif
+
+syn case ignore
+
+set iskeyword=A-Z,a-z
Sorry, I didn't notice - isn't included by default. It should be better now.
zainin
@dkearns , I added the missing keywords.
For the detection I added a regex as well since RouterOS adds a line like this during config export:
# nov/05/2021 22:01:24 by RouterOS 6.41.3
codecov[bot]
Codecov Report
Merging #9097 (23e0fa7) into master (15d9890) will decrease coverage by
87.68%.
The diff coverage isn/a.
@@ Coverage Diff @@ ## master #9097 +/- ## =========================================== - Coverage 90.13% 2.45% -87.69% =========================================== Files 151 149 -2 Lines 169289 166371 -2918 =========================================== - Hits 152594 4089 -148505 - Misses 16695 162282 +145587
| Flag | Coverage Δ | |
|---|---|---|
| huge-clang-none | ? |
|
| huge-gcc-none | ? |
|
| huge-gcc-testgui | ? |
|
| huge-gcc-unittests | 2.45% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/float.c | 0.00% <0.00%> (-99.22%) |
⬇️ |
| src/gui_gtk_f.c | 0.00% <0.00%> (-97.43%) |
⬇️ |
| src/crypt_zip.c | 0.00% <0.00%> (-97.06%) |
⬇️ |
| src/cmdhist.c | 0.00% <0.00%> (-97.03%) |
⬇️ |
| src/sha256.c | 0.00% <0.00%> (-96.94%) |
⬇️ |
| src/evalbuffer.c | 0.00% <0.00%> (-96.88%) |
⬇️ |
| src/textprop.c | 0.00% <0.00%> (-96.84%) |
⬇️ |
| src/match.c | 0.00% <0.00%> (-96.76%) |
⬇️ |
| src/libvterm/src/rect.h | 0.00% <0.00%> (-96.56%) |
⬇️ |
| src/evalfunc.c | 0.00% <0.00%> (-96.51%) |
⬇️ |
| ... and 137 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered by Codecov. Last update 15d9890...23e0fa7. Read the comment docs.
Christian Brabandt
please also update .github/CODEOWNERS
zainin
@chrisbra , sure, done.
Doug Kearns
Would this filetype be better named as routeros? As far as I can tell it's mostly known as RouterOS Script or sometimes MikroTik Script.
Doug Kearns
@dkearns commented on this pull request.
> @@ -11,6 +11,8 @@ endif syn case ignore +syn iskeyword @,-
You've missed digits used in, for example, toip6. So at a minimum I think you'll need:
syn iskeyword @,48-57,-
zainin
@zainin commented on this pull request.
You're right, fixed.
zainin
Would this filetype be better named as routeros?
Makes sense, I didn't actually realize it's named like this in other editors.
I think we can change this after we take care of the other PR you sent me to avoid conflicts.
Bram Moolenaar
If we are going to use "routeros" we better do this right away, no point in first adding rsc and then renaming.
Doug Kearns
Yes, we're just working on a couple of changes in zainin's fork under the old name then it'll be updated to the new name before it's offered for merging here.
zainin
@zainin pushed 8 commits.
- e9209f6 Add escape sequence highlighting
- 9eb332b Add and/or/in operator highlighting
- b7035ef Add new rsc filetype plugin
- cd0339a Exclude leading whitespace from comment highlighting
- 193baed Remove rscSpecial delimiter highlighting from strings
- 584d729 Highlight the line continuation character
- cda2ae6 Rename rsc filetype to routeros
- 58105d2 Merge pull request #1 from dkearns/improve-rsc-syntax-highlighting
—
You are receiving this because you are subscribed to this thread.
zainin
I think we're done for now, let me know if there's anything else to tweak.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub.
Bram Moolenaar
Thanks, I'll include it. The filetype detection will be a patch, the rest will come with the next runtime file update.