jsval is a deprecated typedef for JS::Value as seen in https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_Reference/JS::Value#jsval_is_not_particularly_type-safe
So let's our codebase consistently use JS::Value.
Details
Details
- Reviewers
• leper - Commits
- rP20062: Replace deprecated jsval with JS::Value.
Check the link, apply the patch, make sure that it compiles, search for "jsval" and see it's complete.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Executing section Default... Executing section Source... Executing section JS... Executing section XML GUI... Executing section Python... Executing section Perl...
http://jw:8080/job/phabricator_lint/442/ for more details.
Comment Actions
So basically https://github.com/leper/0ad/commit/408a4201c5530863444c1281ff2a28a9aa793f8e ?
You seem to be missing a few comments, and you might have one more than that diff, ack if you check that everything in that one diff is in this one.
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
http://jw:8080/job/phabricator/1922/ for more details.
Comment Actions
Yep. I reviewed that and updated my diff:
- Entity.h JS int values, i.e. 1-2^30 .. 2^30-1 (inclusive) is misleading. Without refering to JSVAL_INT_MIN JSVAL_INT_MAX`, that comment seems wrong as it is expected that JS Numbers are safe https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isSafeInteger . (The original comment stems from rP7276.) (See following remark).
- test_ScriptConversions.h int JS values why isn't that value capitalized? We want to name the actual type and it actually is a JS::Value this is refering to https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_Reference/JS::Value , an integer JS::Value, not a double one (nor a JS Number, those are greater.)
- DebugSerializer.h nice typo
- ParamNode.h agree to rename these occurences because it names the return type. It is a JS::MutableHandleValue, so stating it returns a JS::Value representation is correct.
- MapGenerator.h: leads to CMapGeneratorWorker::ExportMap that stores a a JS::HandleValue, but since that is JS::Handle<JS::Value>, that's not false either.
- JSInterface_GUITypes.h might just as well fix the painful whitespace on that line
- Besides jsval and jsvals in the comments, there are also js val, js vals and js values occurrences, might just as well make it consistent, JS::Value.
Thanks for the feedback.
Comment Actions
- Entity.h JS int values, i.e. 1-2^30 .. 2^30-1 (inclusive) is misleading. Without refering to JSVAL_INT_MIN JSVAL_INT_MAX`, that comment seems wrong as it is expected that JS Numbers are safe https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isSafeInteger . (The original comment stems from rP7276.) (See following remark).
It is wrong regarding those being stored as integers, but that range is still representable without any missing integers, so the intention is still correct. Also those should be read as JSVAL of INT_MIN, not JSVAL_FOO, which is also why we test for INT_MAX+1 and get back the correct thing.
- JSInterface_GUITypes.h might just as well fix the painful whitespace on that line
Now the macro itself is slightly inconsistent regarding that, but more than one trailing blank for those is ugly anyway.
Should have also fixed the indentation (as in reduced to start with one \t for everything after #define foo(...) \
Thanks for the feedback.
It seems like I cannot accept this anymore, ah well, phabricator's fault only.