fix: silent precendence #455
Conversation
czosel
force-pushed
the
fix-silent-precedence
branch
from
78d1775 to
ec33e9f
Compare
czosel
force-pushed
the
fix-silent-precedence
branch
from
ec33e9f to
05b91b7
Compare
src/ast.js
Outdated
| buffer = result.what; | ||
| result.what = result.what.left; | ||
| this.swapLocations(result, result, result.what, parser); | ||
| const subject = result.kind === "cast" ? "what" : "expr"; |
There was a problem hiding this comment.
could be great to rename what property to expr on cast nodes, that way would be more consistent grammatically speaking - the next release will be stable so it's the last chance to introduce breaking changes.
There was a problem hiding this comment.
I was hoping you'd suggest that 😉 done!
src/ast.js
Outdated
| (result.kind === "silent" && | ||
| result.expr && | ||
| !result.expr.parenthesizedExpression) | ||
| ["silent", "cast"].includes(result.kind) && |
There was a problem hiding this comment.
sorry, I'm a bit maniac, could you rewrite the condition to be more optimized as this function is runned over all nodes it may impact a bit :
(result.kind == 'silent' || result.kind == 'cast') && ...
NB : The property check costs less than a function call and a lookup because you have only 2 entries
There was a problem hiding this comment.
Fixed! Might be a good idea to add performance testing in the future 😉
There was a problem hiding this comment.
it has something similar :
PASS test/perf.test.js
● Console
console.log test/perf.test.js:42
+ Tokens speed => 205.15K/sec
console.log test/perf.test.js:56
+ Reading speed => 437.1K/sec
console.log test/perf.test.js:101
+ Nodes speed => 79.69K/sec
console.log test/perf.test.js:115
+ Overall speed => 211.43K/sec
It's not kilobytes but thousands of tokens or nodes dependending on what is tested.
The problem is that depends on the CPU & RAM frequency (disk does not enter into account) so you can't really know automatically it you break/degrade something - any idea is welcome
There was a problem hiding this comment.
Nice! I have no experience with performance testing myself, but I saw that in prettier/prettier something similar is done based on Benchmark.js.
czosel
force-pushed
the
fix-silent-precedence
branch
from
25951da to
d46ad9c
Compare
czosel
force-pushed
the
fix-silent-precedence
branch
from
d46ad9c to
f03c3e7
Compare
|
@ichiriac What do you think, is this ready to be merged? 🚀 |
Change
silentto be handled exactly the same way ascastis handled. Also refactorshouldBeSamefor better error reporting.Fixes #195
Fixes #355