Finish read-only parity of hex editor #314
Conversation
|
|
connor4312
force-pushed
the
streaming-2
branch
from
2014a8f to
16e046d
Compare
|
Forgot to push my last commit, fixed 😬 |
connor4312
force-pushed
the
streaming-2
branch
from
bcdfb54 to
7503240
Compare
There was a problem hiding this comment.
I would love to see more comments / explanation around all the special math. I also wasn't great about that and feel like that's the main barrier to the hex editors maintainability. There's a lot of thinking that needs to be done to adjust for all the virtualization and sizing. Why are some things floored vs ceiled? Why are some things -1? Things like this will be forgotten in a couple months
| it("round trips", async () => { | ||
| const backup = new Backup(await getTempFile()); | ||
| await backup.write(edits); | ||
| expect(await backup.read()).to.deep.equal(edits); |
There was a problem hiding this comment.
Is there a benefit to chai over mocha's built in assert.strictDeepEqual?
There was a problem hiding this comment.
For strictDeepEqual, not so much, but Chai has additional assertions which makes writing tests easier and/or produces higher quality assertion messages, as well as a plugin API. I don't feel super strongly about it though so I'm fine using just assert if you'd prefer.
There was a problem hiding this comment.
If you think chai will be beneficial in the future let's keep it. However, do you think we should be using Jest and React's testing library https://testing-library.com/docs/react-testing-library/intro/ so we can test component rendering and those sorts of things?
|
Some math has since changed and moved, so I added comments in bcfec6a to the relevant sections. |
* wip * fixup, get replacement editing working * unsaved ranges and go to offset * chore: remove dead code * feat: implement virtual scroll * wip * refactor: add comments and clean up math * find/replace * feat: undo/redo * fix: focus issues with go to offset quickpick * fix: focus back to the byte when hiding find widget * fix: binary in search, cap results by default for perf, progress * fix: don't change selected toolbar background on hover * fix: issues with length changes and large find/replace * feat: add replace toggle * feat: show better validation for find widget errors * feat: wip on string search with placeholders This approach will not work, it's wildcards and not placeholders. But committing it if we need it in the future... * feat: support placeholders in search * feat: persist webview state information * fix: make revert work
This finishes the read-only side of the hex editor. Supports keyboard selection, the data inspector, multiple ranges, all that...
Recapping from what I discussed in standup, the basic model is that there's a scroll container that is sized and just uses native scrolling. Inside that is the sticky-positioned data view (sticky, since setting it to fixed prevents scrolling from propagating). This is a static display of data. Only rows in the view are rendered, and as scroll happens the rows are moved, added, and removed.
I mentioned that I was having some performance issues with Recoil here. I've got this so that it's now okay. It's now 'okay'. Selection updates take about 5ms on my macbook, and scrolling takes about 8ms. It's not great but acceptable. If it ends up being problematic we can look at optimizing it; current usage is relatively idiomatic React however.
Note that when testing or evaluating performance, you want to run
npm run watch -- --minifyto create a production build. The performance characteristics of React when built for production are significantly better than in development.