feat(hints): allow custom hint generator functions #80
Conversation
This lets you override the prefix-free generator with a custom function (e.g., to make a sequential numeric generator).
The function is only a few lines, but we can help out users by providing a stock implementation. Note that we ignore hint_chars here! You might think that we could provide just a "sequential" generator that would turn an alphabet like "abc" into: a, b, c, aa, ab, ac, ba, bb, bc, ca, cb, cc, aaa, ... and then feed it the numeric digits via hint_chars. But that is not quite how numbers work. If the alphabet is "1234567890", then we get "0" instead of "10" as the tenth label, and "10" as the twentieth. If you instead use "0123456789", then we start with "0" and get "01", "02", etc, instead of the teens. You'd have to be smart about how zeroes are handled, at which point it is easier to just have a custom generator that simply counts.
There was a problem hiding this comment.
This is very cool, thank you!
My only concern with the current API shape is that it limits the sources for pulling in labels. As in, this doesn't let you define labels based on specific Element attributes, e.g. using the text of the element as a label.
My current thinking is that this could just happen in the pick function, where we'd add support for filling in a label property and then respecting that, e.g. along the lines of
glide.hints.show({
pick(hints) {
hints[0].label = "a";
return hints;
},
});But this feels pretty hacky and does complicate the internal label generator process a lot, as we'd need to make sure the labels the user set don't overlap with the generated labels. (or just require all hints to be given a label, if you want this behaviour)
So I've been trying to think about a better approach but haven't come up with anything yet... do you have any ideas for how this could be integrated?
I'm asking as figuring this out may result in a breaking change to the API introduced in this PR, so ideally we'd figure out the design for content-process label generation before shipping this.
| // but we haven't made the api object yet! This may be an indication that | ||
| // the helper functions should go somewhere else (and maybe just | ||
| // get aliased into the api so users can see them). | ||
| hint_label_generator: (alphabet: string[], len: number) => string[]; |
There was a problem hiding this comment.
Ah to add a new glide.o option, you need to add it to glide.Options type as well as the internal runtime version.
| // Ideally this would default to glide.api.hints.label_prefix_free, | ||
| // but we haven't made the api object yet! This may be an indication that |
There was a problem hiding this comment.
One option is to use a getter/setter like the above option, so that it's lazily initialised.
e.g.
#hint_label_generator: glide.Options['hint_label_generator'] | null = null;
get hint_label_generator() {
return this.#hint_label_generator ?? GlideBrowser.api.hints.label_prefix_free;
}
set hint_size(value: glide.Options['hint_label_generator']) {
this.#hint_label_generator = value;
}But defining the default label generator in hinting.mts and then just referencing it here would also be a fine approach.
| of the characters in [hint_chars](#glide.o.hint_chars). This is the default. | ||
|
|
||
| - `glide.hints.label_numeric`: Generate sequential numeric labels, | ||
| starting at `1` and counting up. Ignores [hint_chars](#glide.o.hint_chars). |
There was a problem hiding this comment.
Ah this file is auto-generated, so you should move this description to glide.Options and then run pnpm build:docs.
I also just added a header to this file so it should be more obvious that it's auto-generated :)
| // but we haven't made the api object yet! This may be an indication that | ||
| // the helper functions should go somewhere else (and maybe just | ||
| // get aliased into the api so users can see them). | ||
| hint_label_generator: (alphabet: string[], len: number) => string[]; |
There was a problem hiding this comment.
fyi that this type doesn't match the implementations as here the arguments are (alphabet: string[], len: number) but the default generator just accepts len, label_prefix_free(len: number): string[].
2 participants
Following up on the discussion in #58 (reply in thread), this adds a config option to provide a custom hint generator function. And since my goal there was sequential numeric hints, it also provides a stock function to do that.
This is more flexible than just a string-based option name (like "prefix-free" vs "numeric"), since you can provide an arbitrary function. But my hope is that by giving stock functions for those, it ends up being about equally easy for a user who wants to pick one of them.
It did create some headaches for me in the code, though. Where should these stock functions live? I stuck them in
glide.hints.*, but that might not be appropriate. It also creates a challenge as that is not available when we are initializing theglide.ooptions (so we carry a null when the function is unset and fill in the default dynamically at the point of use).I don't usually write typescript or javascript, so everything here is largely cargo-culted from surrounding code. Apologies in advance for any egregious style or idiom violations.