feat: remove dependency on rcedit to allow x-platform exe resource modding #1696
Conversation
MarshallOfSound
force-pushed
the
remove-rcedit
branch
from
3395c89
to
1516698
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1696 +/- ##
==========================================
- Coverage 91.80% 89.29% -2.51%
==========================================
Files 16 17 +1
Lines 854 897 +43
Branches 167 186 +19
==========================================
+ Hits 784 801 +17
- Misses 52 60 +8
- Partials 18 36 +18 ☔ View full report in Codecov by Sentry. |
MarshallOfSound
changed the title
There was a problem hiding this comment.
I took a gander through the resedit source code (https://github.com/jet2jet/resedit-js/blob/main/package.json) and it seems really well done, so I'm generally supportive.
I found some code that looks a little WIP but I think this is overall net-good.
Yeah for context I started out implementing this myself, then hit an edge case I didn't fully understand. In my search for the answer I found an impl that was very similar to what I was writing from scratch so decided to use that impl instead. It's 0 dependencies, and works quite well. |
| throw new Error(`Incorrectly formatted version string: "${str}". Should have at least one and at most four components`); | ||
| } | ||
| return parts.map((part) => { | ||
| const parsed = parseInt(part, 10); |
There was a problem hiding this comment.
| const parsed = parseInt(part, 10); | |
| const parsed = Number(part); |
issue: parseInt has some edge cases like allowing mixed alphanumeric string/numbers. Is that something that we explicitly want to allow in this case?
e.g.
parseInt('123alphanumeric', 10)
// 123There was a problem hiding this comment.
Number has these quirks too Number('0xc') === 12
|
🎉 This PR is included in version 18.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This re-implements everythin that
rceditused to do with a native binary using a purely JS implementation of resource editing.This is required to drop the package-on-platform requirements of the win32 target, it also will make it easier to add asar integrity windows manifest values in the future.