fix linting warnings - #9103
Conversation
Signed-off-by: Vaivaswat <vaivaswat2244@gmail.com>
c8f12be to
8285f24
Compare
ksen0
left a comment
ksen0
left a comment
There was a problem hiding this comment.
Thanks for this, I did some spot checks. If tests all pass then its fine, however I did see changes in the test suite, could you briefly clarify please?
|
@Vaivaswat2244 I was just checking this again, there's a merge conflict now, could you resolve and ping on Discord please (just in the main contribute-to-p5 channel) so we could merge this before more changes and more merge conflicts? Sorry, thank you! |
|
Hi @Vaivaswat2244 ! So sorry I merged this is too fast (I just reverted) - leaving a few comments |
| } | ||
|
|
||
| // Will do conversion in-Gamut as out of Gamut conversion is only really useful for futher conversions | ||
| #toColorMode(mode) { |
There was a problem hiding this comment.
Ok to fix warnings but code should not be removed, you can add a flag o ignore the warning maybe?
ksen0
left a comment
ksen0
left a comment
There was a problem hiding this comment.
Sorry I didn't flag before; I did a few manual tests and overall the cleanups are helpful I think but could you reinstate some of the code that got factored out?
| * Adapted from http://stackoverflow.com/a/25355178 | ||
| * @private | ||
| */ | ||
| function calculateOffset(object) { |
| } | ||
| } | ||
|
|
||
| remove() { |
There was a problem hiding this comment.
I see where this went but it still feels like a refactor
| throw Error('_yAlignOffset: height is required'); | ||
| } | ||
|
|
||
| let { textLeading, textBaseline, textSize, textFont } = this.states; |
|
|
||
| function escapeRegExp(str) { | ||
| return str.replace(/[-\[\]/\{}\()\*+\?.\\^\$|]/g, '\\$&'); | ||
| return str.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&'); |
| * | ||
| * @private | ||
| */ | ||
| function destroyClickedElement(event) { |
| // mat3; // only need to allocate in this rare case | ||
| } else if (multMatrix instanceof MatrixNumjs) { | ||
| _src = multMatrix.mat3; | ||
| // tempMatrix already holds the matrix we need |
| return id; | ||
| } | ||
|
|
||
| function getNodeKey(node) { |
2 participants
Resolves partially: #9070
Changes:
Fixes all 143 oxlint warnings; npm run lint now exits clean. No lint config was changed. Every warning was resolved in code.
Mostly mechanical:
One behaviour change worth review:
Renderer3Ddeclaredremove()twice. The first (canvas teardown, added in 2fa995a) was silently shadowed by a second added later in ef28cbc for_textCanvas, sowrappedEltcleanup has never run. I merged them,Renderer2D.remove()has the identical four lines, which is what it was meant to mirror. WebGL/WebGPU canvases are now actually detached on remove().Two edits look behavioural but aren't: return FES.internalError(...) is unreachable (it always throws), and if (0) → if (debug) in textCore.js uses a param that arrives falsy.
Screenshots of the change:
PR Checklist
Verification
WebGPU visual cases and p5.RendererWebGPU were not run locally cuz no software WebGPU adapter available :) leaving those to CI.