-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Update sqf.js to match latest changes in Arma 3 v2.11 #3686
Conversation
src/languages/sqf.js
Outdated
className: 'meta', | ||
begin: /#\s*[a-z]+\b/, | ||
end: /$/, | ||
keywords: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically this could collapse to just keywords
a nested hash isn't necessary if one isn't specified keyword
is the default scope used.
src/languages/sqf.js
Outdated
// https://community.bistudio.com/wiki/PreProcessor_Commands | ||
const PREPROCESSOR = { | ||
className: 'meta', | ||
begin: /#\s*[a-z]+\b/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to start at the beginning of a line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to start at the beginning of a line?
Whitespaces are allowed too. Will update this too.
src/languages/sqf.js
Outdated
], | ||
illegal: [ | ||
//$ is only valid when used with Hex numbers (e.g. $FF) or at the beginning of localized strings (e.g. "$STR_HELLO") | ||
/\$[^Sa-fA-F0-9]/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the S
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the
S
here?
I wasn't sure if it would flag a string like "$STR_bla"
as illegal. (which are localized strings)
It was just my lazy attempt to prevent it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I yet follow how one knows it's a localized string - they alway have a $STR prefix? If it's actually quoted though then it's already handled by STRINGS rule and doesn't need to be guarded against here since its inside the string
scope already, unless I'm missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I yet follow how one knows it's a localized string - they alway have a $STR prefix? If it's actually quoted though then it's already handled by STRINGS rule and doesn't need to be guarded against here since its inside the
string
scope already, unless I'm missing something?
Yeah if the string rule prevents it I should remove it. I didn't actually test this to be sure.
src/languages/sqf.js
Outdated
'waitUntil', 'while', 'with' | ||
]; | ||
|
||
const LITERAL = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a LOT of literals. Consider variable.constant
and variable.language
perhaps... pi
(if it's math) seems like a reference to a literal (a constant), not a literal itself.
I won't force any changes here since I dunno the language, but just making you aware we have more scopes to differentiate these sorts of things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a LOT of literals. Consider
variable.constant
andvariable.language
perhaps...pi
(if it's math) seems like a reference to a literal (a constant), not a literal itself.I won't force any changes here since I dunno the language, but just making you aware we have more scopes to differentiate these sorts of things.
Technically they're all commands (so they should be part of built-in array). They're what we call "nular (sic) commands" (commands with no left or right params, i.e. they behave like functions with no params or sometimes global variables)
Again this is from previous contributions so I didn't want to change them.
Could you point me to the list of scopes you guys have?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://highlightjs.readthedocs.io/en/latest/css-classes-reference.html
Technically they're all commands (so they should be part of built-in array).
Let's not do that. :-). For first party grammars we take a "wider perspective" view of such things. Literal concepts that are non-reducible like true, false, null, nil, undefined... these are considered literals. We prefer consistency in this across our grammars - rather than adhering to any particular grammars technicalities.
Since pi
is simply a label for 3.1459 etc it would be better thought of as a variable.constant
... and the fact that what provides that is a function pi()
doesn't really change that "wider perspective" categorization.
Your new file has different line endings on most/all lines, resave with same as the previous version |
@@ -2506,7 +2629,7 @@ export default function(hljs) { | |||
hljs.C_BLOCK_COMMENT_MODE | |||
] | |||
}; | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/languages/sqf.js
Outdated
'worldToScreen', | ||
]; | ||
'worldToScreen' | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
]; | |
]; |
@@ -2424,7 +2543,7 @@ export default function(hljs) { | |||
'vestMagazines', | |||
'viewDistance', | |||
'visibleCompass', | |||
'visibleGPS', | |||
'visibleGps', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'visibleGps', | |
'visibleGPS', |
'getUserMFDText', | ||
'getUserMFDValue', | ||
'getVariable', | ||
'getVehicleCargo', | ||
'getVehicleTIPars', | ||
'getVehicleTiPars', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'getVehicleTiPars', | |
'getVehicleTIPars', |
'setTimeMultiplier', | ||
'setTiParameter', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'setTiParameter', | |
'setTIParameter', |
'setVehicleLock', | ||
'setVehiclePosition', | ||
'setVehicleRadar', | ||
'setVehicleReceiveRemoteTargets', | ||
'setVehicleReportOwnPosition', | ||
'setVehicleReportRemoteTargets', | ||
'setVehicleTIPars', | ||
'setVehicleTiPars', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'setVehicleTiPars', | |
'setVehicleTIPars', |
@@ -2147,7 +2269,7 @@ export default function(hljs) { | |||
'showCommandingMenu', | |||
'showCompass', | |||
'showCuratorCompass', | |||
'showGPS', | |||
'showGps', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'showGps', | |
'showGPS', |
src/languages/sqf.js
Outdated
'sideLogic', | ||
'sideUnknown', | ||
'sideEnemy', | ||
'sideFriendly', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not in alphabetic order
I'm not gonna update this PR beyond this point, because it took way more time that I wanted to invest in it. The point was to update the SQF commands list and fix the highlighter breaking when you put Further changes to other stuff can be made in future contributions. Regarding the spelling changes, like I said they come from the game itself, not the wiki. I prefer to keep it as the game says, which is more correct. |
Indeed, |
1a211e7
to
47daaf3
Compare
Whatever you did with line endings made this almost impossible to rebase cleanly. I finally just gave up and went with |
Changes
#
as an illegal character (legal as of Arma 3 v1.82)This is a follow up to #3193. It was quite outdated so I opened a new one. It also applies the requested changes to make it not fail the detection asserts.
P.S: I think GitHub's diff is broken because it shows the entire file has changed. It looks correct in VSCode though.