Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions src/glide/browser/base/content/browser-commands.mts
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,9 @@
return;
}

const hint_chars = GlideBrowser.api.options.get("hint_chars");
const hint_keys = GlideBrowser.api.keymaps.list("hint").map((k) => k.lhs);

const alphabet = hint_keys.length
? hint_chars.split("").filter((k) => !hint_keys.includes(k))
: hint_chars.split("");
const labels = Strings.generate_prefix_free_codes(alphabet, hints.length, this.#make_alphabet_cost_map(hint_chars));
const hint_gen = GlideBrowser.api.options.get("hint_label_generator")

Check failure on line 168 in src/glide/browser/base/content/browser-commands.mts

View workflow job for this annotation

GitHub Actions / lint

Argument of type '"hint_label_generator"' is not assignable to parameter of type 'keyof Options'.
|| GlideBrowser.api.hints.label_prefix_free;

Check failure on line 169 in src/glide/browser/base/content/browser-commands.mts

View workflow job for this annotation

GitHub Actions / lint

Property 'label_prefix_free' does not exist on type '{ show(opts?: { selector?: string | undefined; include?: string | undefined; editable?: boolean | undefined; auto_activate?: boolean | undefined; action?: "click" | "newtab-click" | ((target: HTMLElement) => Promise<void>) | undefined; location?: HintLocation | undefined; pick?: ((hints: ContentHint[]) => ContentHint[]) | undefined; } | undefined): void; }'.
const labels = hint_gen(hints.length);

for (let i = 0; i < hints.length; i++) {
const hint = hints[i]!;
Expand Down Expand Up @@ -210,6 +206,15 @@
return cost_map;
}

hint_label_prefix_free(len: number): string[] {
const hint_chars = GlideBrowser.api.options.get("hint_chars");
const hint_keys = GlideBrowser.api.keymaps.list("hint").map((k) => k.lhs);
const alphabet = hint_keys.length
? hint_chars.split("").filter((k) => !hint_keys.includes(k))
: hint_chars.split("");
return Strings.generate_prefix_free_codes(alphabet, len, this.#make_alphabet_cost_map(hint_chars));
}

async #create_commandline(tab: BrowserTab, opts: { prefill?: string } = {}) {
let browser = gBrowser.getBrowserForTab(tab);

Expand Down
19 changes: 19 additions & 0 deletions src/glide/browser/base/content/browser.mts
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,12 @@
this.#hint_size = value;
GlideBrowser.set_css_property("--glide-hint-font-size", value);
}

// 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
Comment on lines +1669 to +1670
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// 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[];

Check failure on line 1673 in src/glide/browser/base/content/browser.mts

View workflow job for this annotation

GitHub Actions / lint

Property 'hint_label_generator' has no initializer and is not definitely assigned in the constructor.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah to add a new glide.o option, you need to add it to glide.Options type as well as the internal runtime version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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[].

}

function make_glide_api(): typeof glide {
Expand Down Expand Up @@ -1857,7 +1863,20 @@
debug: Services.prefs.getBoolPref("devtools.testing", false),
});
},

label_prefix_free(len: number): string[] {

Check failure on line 1867 in src/glide/browser/base/content/browser.mts

View workflow job for this annotation

GitHub Actions / lint

Object literal may only specify known properties, and 'label_prefix_free' does not exist in type '{ show(opts?: { selector?: string | undefined; include?: string | undefined; editable?: boolean | undefined; auto_activate?: boolean | undefined; action?: "click" | "newtab-click" | ((target: HTMLElement) => Promise<void>) | undefined; location?: HintLocation | undefined; pick?: ((hints: ContentHint[]) => ContentHint[]) | undefined; } | undefined): void; }'.
return GlideCommands.hint_label_prefix_free(len);
},

label_numeric(len: number): string[] {
var ret = [];
for (var i = 1; i <= len; i++) {
ret.push(i.toString());
}
return ret;
},
},

addons: {
async install_from_url(xpi_url): Promise<glide.Addon> {
const installer = await AddonManager.getInstallForURL(xpi_url);
Expand Down
41 changes: 41 additions & 0 deletions src/glide/browser/base/content/test/hints/browser_hints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,44 @@
await keys("<esc>");
});
});

add_task(async function test_hint_generator_config() {
await GlideTestUtils.reload_config(function _() {
glide.o.hint_label_generator = (len) => {

Check failure on line 276 in src/glide/browser/base/content/test/hints/browser_hints.ts

View workflow job for this annotation

GitHub Actions / lint

Parameter 'len' implicitly has an 'any' type.

Check failure on line 276 in src/glide/browser/base/content/test/hints/browser_hints.ts

View workflow job for this annotation

GitHub Actions / lint

Property 'hint_label_generator' does not exist on type 'Options'.
return ["foo", "bar", "baz"].slice(0, len);
};
glide.keymaps.set("normal", "f", () => {
glide.hints.show({
pick: (hints) => hints.slice(0, 2),
});
});
});

await BrowserTestUtils.withNewTab(FILE, async _browser => {
await keys("f");
await wait_for_hints();

const hints = GlideCommands.get_active_hints();
is(hints[0]?.label, "foo");
is(hints[1]?.label, "bar");
is(hints.length, 2);
await keys("<esc>");
});
});

add_task(async function test_numeric_hint_generator() {
await GlideTestUtils.reload_config(function _() {
glide.o.hint_label_generator = glide.hints.label_numeric;

Check failure on line 300 in src/glide/browser/base/content/test/hints/browser_hints.ts

View workflow job for this annotation

GitHub Actions / lint

Property 'label_numeric' does not exist on type '{ show(opts?: { selector?: string | undefined; include?: string | undefined; editable?: boolean | undefined; auto_activate?: boolean | undefined; action?: "click" | "newtab-click" | ((target: HTMLElement) => Promise<void>) | undefined; location?: HintLocation | undefined; pick?: ((hints: ContentHint[]) => ContentHint[]) | undefined; } | undefined): void; }'.

Check failure on line 300 in src/glide/browser/base/content/test/hints/browser_hints.ts

View workflow job for this annotation

GitHub Actions / lint

Property 'hint_label_generator' does not exist on type 'Options'.
});

await BrowserTestUtils.withNewTab(FILE, async _browser => {
await keys("f");
await wait_for_hints();

const hints = GlideCommands.get_active_hints();
is(hints[0]?.label, "1");
is(hints[1]?.label, "2");
is(hints[9]?.label, "10");
await keys("<esc>");
});
});
12 changes: 12 additions & 0 deletions src/glide/docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ text-decoration: none;
[`glide.o.jumplist_max_entries`](#glide.o.jumplist_max_entries)\
[`glide.o.hint_size`](#glide.o.hint_size)\
[`glide.o.hint_chars`](#glide.o.hint_chars)\
[`glide.o.hint_label_generator`](#glide.o.hint_label_generator)\
[`glide.bo`](#glide.bo)\
[`glide.options`](#glide.options)\
[`glide.options.get()`](#glide.options.get)\
Expand Down Expand Up @@ -206,6 +207,17 @@ The characters to include in hint labels.

`ts:@default "hjklasdfgyuiopqwertnmzxcvb"`

### `glide.o.hint_label_generator: (len: number) => string[]` {% id="glide.o.hint_label_generator" %}

A function to produce labels for `len` hints. You can provide your own
function or use an included one:

- `glide.hints.label_prefix_free`: Generate prefix-free combinations
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).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)


## • `glide.bo: Partial<glide.Options>` {% id="glide.bo" %}

Set buffer specific options.
Expand Down
2 changes: 2 additions & 0 deletions src/glide/docs/hints.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ If there are too many hints for each label to have a single character from the a
- Elements 2-26 get single-character labels: `j`, `k`, `l`, `a` etc.
- Elements 1 & 27-30 get two-character labels: `hh`, `hj`, `hk`, `hl`

The algorithm above is implemented by the `glide.hints.label_prefix_free` generator. You can provide your own generator function by setting [glide.o.hint_label_generator](api.md#glide.o.hint_label_generator).

## Default key mappings

| Key | Action |
Expand Down
Loading