Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make NormalizedHotkeyString extend NormalizedSequenceString #106

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

iansan5653
Copy link
Member

@iansan5653 iansan5653 commented Oct 12, 2023

A HotkeyString is a valid SequenceString but with only one item in the sequence. So anything that accepts SequenceString should also accept HotkeyString.

This PR makes HotkeyString extend SequenceString to allow for this.

This felt a little backwards to me because sequences are built from hotkeys, not the other way around, but it is correct. I made a simple demo to test it out:

const normalizedHotkeyBrand = Symbol('normalizedHotkey')
export type NormalizedHotkeyString = NormalizedSequenceString & {[normalizedHotkeyBrand]: true}

const normalizedSequenceBrand = Symbol('normalizedSequence')
export type NormalizedSequenceString = string & {[normalizedSequenceBrand]: true}

const hotkey = "a" as NormalizedHotkeyString
const sequence = "a b" as NormalizedSequenceString

const thingsThatAcceptSequencesShouldAcceptHotkeys = (sequence: NormalizedSequenceString) => {}
thingsThatAcceptSequencesShouldAcceptHotkeys(sequence)
thingsThatAcceptSequencesShouldAcceptHotkeys(hotkey)

const thingsThatAcceptHotkeysShouldNotAcceptSequences = (hotkey: NormalizedHotkeyString) => {}
thingsThatAcceptHotkeysShouldNotAcceptSequences(hotkey)
// @ts-expect-error
thingsThatAcceptHotkeysShouldNotAcceptSequences(sequence)

Note

Using string template types for this would solve this problem more elegantly, but that's a lot more complicated and comes with more of a compile-time performance cost. This is 'good enough'.

@iansan5653 iansan5653 requested a review from a team as a code owner October 12, 2023 16:55
@iansan5653 iansan5653 requested a review from dgreif October 12, 2023 16:55
@iansan5653 iansan5653 changed the title Make NormalizedHotkeyString extend NormalizedHotkeyString Make NormalizedHotkeyString extend NormalizedSequenceString Oct 12, 2023
@iansan5653 iansan5653 force-pushed the hotkey-extend-sequence branch from d68cf51 to 402ae34 Compare October 12, 2023 16:58
@iansan5653 iansan5653 merged commit dd45427 into main Oct 12, 2023
2 checks passed
@iansan5653 iansan5653 deleted the hotkey-extend-sequence branch October 12, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants