-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
refactor(generators): Migrate Lua generators to TypeScript #7654
Conversation
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.
Approved, with just a few minor tweaks to comments / formatting.
generators/lua/lua_generator.ts
Outdated
@@ -5,40 +5,38 @@ | |||
*/ | |||
|
|||
/** | |||
* @fileoverview Helper functions for generating Lua for blocks. | |||
* @file Helper functions for generating Lua for blocks. |
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.
For consistency with the other CodeGenerator subclasses:
* @file Helper functions for generating Lua for blocks. | |
* @file Lua code generator class, including helper methods for | |
* generating Lua for blocks. |
(But now I've noticed that I forgot to replace @fileoverview
with @file
in lots of places in the other generators…)
generators/lua/lua_generator.ts
Outdated
import {inputTypes} from '../../core/inputs/input_types.js'; | ||
|
||
|
||
/** | ||
* Order of operation ENUMs. | ||
* http://www.lua.org/manual/5.3/manual.html#3.4.8 | ||
* @enum {number} |
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.
The @enum
JSDoc tag is no longer needed and should be deleted.
generators/lua/lua_generator.ts
Outdated
HIGH = 1, // Function calls, tables[] | ||
EXPONENTIATION = 2, // ^ | ||
UNARY = 3, // not # - ~ | ||
MULTIPLICATIVE = 4, // * / % | ||
ADDITIVE = 5, // + - | ||
CONCATENATION = 6, // .. | ||
RELATIONAL = 7, // < > <= >= ~= == | ||
AND = 8, // and | ||
OR = 9, // or |
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.
Please restore the original formatting here and use // prettier-ignore
to preserve it.
Thanks for merging this and resolving the small tweaks after! I was out last week, so I would've have been able to get to it until this morning. |
In PRs google#7602, google#7616, google#7646, google#7647 and google#7654 the @Protected access modifier on scrub_ was not transcribed to the new typescript signature. BREAKING CHANGE: This makes scrub_ protected again, which will break any code attempting to access this method from outside a CodeGenerator subclass.
Fixes google#2156. In PRs google#7602, google#7616, google#7646, google#7647 and google#7654 the @Protected access modifier on scrub_ on the CodeGenerator subclasses was not transcribed to the new typescript signature. I was going to re-add it, but this breaks some of the procedure block generator functions which rely on it, and then @BeksOmega pointed out that this might be one of the CodeGenerator API functions which we had already decided should be public—and lo and behold I found google#2156. Per discussion amongst team, I am not renaming it to scrub at this time.
Fixes google#2156. In PRs google#7602, google#7616, google#7646, google#7647 and google#7654 the @Protected access modifier on scrub_ on the CodeGenerator subclasses was not transcribed to the new typescript signature. I was going to re-add it, but this breaks some of the procedure block generator functions which rely on it, and then @BeksOmega pointed out that this might be one of the CodeGenerator API functions which we had already decided should be public—and lo and behold I found google#2156. Per discussion amongst team, I am not renaming it to scrub at this time.
Fixes google#2156. In PRs google#7602, google#7616, google#7646, google#7647 and google#7654 the @Protected access modifier on scrub_ on the CodeGenerator subclasses was not transcribed to the new typescript signature. I was going to re-add it, but this breaks some of the procedure block generator functions which rely on it, and then @BeksOmega pointed out that this might be one of the CodeGenerator API functions which we had already decided should be public—and lo and behold I found google#2156. Per discussion amongst team, I am not renaming it to scrub at this time.
Fixes #2156. In PRs #7602, #7616, #7646, #7647 and #7654 the @Protected access modifier on scrub_ on the CodeGenerator subclasses was not transcribed to the new typescript signature. I was going to re-add it, but this breaks some of the procedure block generator functions which rely on it, and then @BeksOmega pointed out that this might be one of the CodeGenerator API functions which we had already decided should be public—and lo and behold I found #2156. Per discussion amongst team, I am not renaming it to scrub at this time.
The basics
The details
Resolves
Part of #6828.
Proposed Changes
Migrate
generators/lua/*.js
andgenerators/lua.js
to TypeScript.Reason for Changes
Finish TS migration.
Test Coverage
npm test
passes. No changes to manual test procedures required.Documentation
No changes to documentation yet, as there should be minimal changes to the generated code and no changes to the published typings (because during build we overwrite the generated
.d.ts
files with the ones fromtypings/
).Additional Information
It may be easier to review this commit-by-commit, as the final format commit make a lot of noisy changes.