Incrementally Improved Input Example - #7
Conversation
dreyfus92
left a comment
dreyfus92
left a comment
There was a problem hiding this comment.
first batch of comments, i will resume my review later on 👀 but very interesting awesome job Jacob 😄
| push(events); | ||
| if (pending) return; |
There was a problem hiding this comment.
here you're not adding the clearTimeout that 2-parser through 4-modes had. if i'm understanding this properly, a flush timer is outstanding and a new chunk arrives, if(pending) return drops the new scan's pending hint and the stale timer still fires and flush-scans w/e partial seq is in the parseer at that point. a lone ESC -> timer scheduled would be a concrete repro. ~20 ms later the first bytes of an arrow key land in their own chunk, stale timer fires and flushes the half-received arrow seq as garbage.
| const flush = input.scan(); | ||
| push(flush.events); | ||
| pending = flush.pending; | ||
| if (pending) processChunk(new Uint8Array()); |
There was a problem hiding this comment.
this recursive call is a no-op that deadlocks, the callback just set pending = flush.pending (truthy), so the recursive call immediately hits if(pending) return no new timer gets scheduled and the parser sits on buffered state until the next real keypress.
| function processChunk(buf: Uint8Array): void { | ||
| const { events, pending: p } = input.scan(buf); | ||
| push(events); | ||
| if (pending) return; |
There was a problem hiding this comment.
same pending timer issue as 5-stream, line 117
|
|
||
| // Note that our state is still reactive, but holds the sole ownership is triggering a render. | ||
| // But how would you handle events which require a render, but aren't directly attached to a state change? | ||
| // See ./6-mouse.ts for an example of handling events from a mouse. |
There was a problem hiding this comment.
| // See ./6-mouse.ts for an example of handling events from a mouse. | |
| // See ./6-events.ts for an example of handling events from a mouse. |
stale ref i believe?
| if (event.ctrl && event.code === 'c') return { ...s, quit: true }; | ||
| if (event.code === 'Escape') return { ...s, quit: true }; | ||
| if (event.code === 'Tab') return { ...s, focusIndex: (s.focusIndex + 1) % 2 }; | ||
| if (event.code === 'Backtab') return { ...s, focusIndex: (s.focusIndex + 1) % 2 }; |
There was a problem hiding this comment.
this seems to be working because n === 2 , but Backtab should go backwards (s.focusIndex - 1 + n) % n anyone copying this for 3+ panels might get silently wrong focus order.
2 participants
What does this PR do?
This provides multiple examples showing the iteration and development of features on top of our current stack of packages. The intent is that we can further discuss and communicate the value of each feature, be it either current or future.
We may chose to add features to the
@clack/uiproject which would eliminate the need for the vendored version, or reduce/remove some of our code. We can update these over time and possibly collapse the number of examples if our helper functions in the future eliminate the need / differences between a "level".Type of change
Checklist
pnpm test)pnpm format)AI-generated code disclosure