Fix keyscheme common settings

[aadcg]

Description

A hot fix solution until issues #2351 and #2296 are solved since the current state is hurting our users (see #3096).

Discussion

I believe that (define-configuration buffer ...) is superior than (define-configuration (web-buffer prompt-buffer) ...) since the keyscheme should be propagated to all classes (including user defined ones) inheriting from buffer.

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

• Git hygiene:
  • I have pulled from master before submitting this PR
  • There are no merge conflicts.
• I've added the new dependencies as:
  • ASDF dependencies,
  • Git submodules,

    cd /path/to/nyxt/checkout
    git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser

  • and Guix dependencies.
• My code follows the style guidelines for Common Lisp code. See:
  • Norvig & Pitman's Tutorial on Good Lisp Programming Style (PDF)
  • Google Common Lisp Style Guide
• I have performed a self-review of my own code.
• My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
• Documentation:
  • All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
  • I have updated the existing documentation to match my changes.
  • I have commented my code in hard-to-understand areas.
  • I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
    • Changelog update should be a separate commit.
  • I have added a migration.lisp entry for all compatibility-breaking changes.
  • (If this changes something about the features showcased on Nyxt website) I have these changes described in the new/existing article at Nyxt website or will notify one of maintainters to do so.
• Compilation and tests:
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works. (If possible.)
  • I ran the tests locally ((asdf:test-system :nyxt) and (asdf:test-system :nyxt/gi-gtk)) and they pass.

[jmercouris]

I even got tripped up on this, like the other user, I was wondering why my Emacs bindings didn't work!

[Ambrevar]

This issue was possibly introduced with e5c8920.

@aartaka Why did you switch from input-buffer to web-buffer?

@aadcg I suggest to use input-buffer in this hot fix.

[aadcg]

@aadcg I suggest to use input-buffer in this hot fix.

Good call. Updated.

[aartaka]

@aartaka Why did you switch from input-buffer to web-buffer?

Because emacs-mode/vi-mode is a keyscheme-mode, and, by extension, a mode. Which means it only works in modable-buffer. While it's conceptually appealing to use input-buffer, and it's unlikely to break core of Nyxt... It breaks one of the premises of this exact PR:

keyscheme should be propagated to all classes (including user defined ones) inheriting from buffer

To avoid breaking things downstream by applying the incorrect mixin, I've switched to web-buffer, because it seems to be the smallest proper class inheriting from both input-buffer and modable-buffer.

[aartaka]

Leaving input-buffer/modable-buffer/web-buffer bug aside, I'm still cautious about imposing all the keyschemes on all the "input" buffers. Regular buffers and prompt buffers (I'm setting panel buffers aside for now) are different in modality, and forcing the same kescheme on both is not always suitable.

Instead of the direction this PR takes, I'd suggest splitting the web/prompt buffer configuration into two separate sections. So that it's:

# Regular buffer keyscheme (keyboard shortcuts)
- CUA
- Emacs
- VI

# Prompt buffer (input menu) keyscheme
- CUA
- Emacs
- VI command
- VI insert

Notice that VI insert is the prompt-buffer-specific option—it's been requested in the past, and, while mostly fixed with input focus mode switching, might still prove useful to force when in prompt buffer.

---

Having both web and prompt buffers to have the same keyscheme takes away flexibility and can be harmful in some corner-cases (I myself want to switch to VI bindings for regular buffers, but I'll never surrender Emacs mode in the prompt buffer, because Emacs input scheme is a perfect match for a mode-less input area with instant feedback).

On the opposite, having separate prompt buffer configuration makes the behavior more flexible and makes it clear that there are two really different modalities one should consider separately. We lose nothing (except the screen space another <select> might take), while gaining a more explicit interface and highlighting modality difference. Win-win, in other words.

[mmahmoudian]

I just realized this PR exists, so I'm going to put my two cents in, hoping you don't mind.

I don't know the ins and outs of this project to the extent to be confident enough about my understanding of the change or the terminology that was discussed. These said, I believe I have understood the code change, and also the comment of @aartaka . As per my understanding, his comment is in-line with my suggestion . Here is a rephrase of my suggestion:

In the settings page, allow user to choose the default keybindings (in @aartaka terminology "regular buffer"), and also add another drop-down menu for selecting the keybinding scheme for the "prompt buffer". Upon selection of the "regular buffer" binding scheme:

1. change the "prompt buffer" binding scheme to the appropriate type (based on that particular Editor's behavior) so that users feel like home.
2. right in front of it in a yellow box, inform the user that the "prompt buffer" scheme is now changed to match based on their selection, but they can change if if they want to

Something like this mockup can make it clear to users what each option is:

This way there is no surprise, and new users would feel like home and also people like me would highly appreciate the customizability and flexibility of Nyxt. 🤓

[Ambrevar]

I agree with @aartaka and @mmahmoudian .

[aadcg]

@jmercouris, thoughts?

[jmercouris]

I don't care, just make it obvious for the user what is going to happen! :-D

[aadcg]

I'm not convinced by the argument that proposes separating setting the keyscheme for web-buffer and prompt-buffer. From the user's perspective, using technical jargon such as prompt buffer in the common-settings (a page where they may have just landed after installing Nyxt) is not wise.

From a technical perspective, the separation isn't exhaustive as well. Think of editor-buffer and note that it doesn't inherit from either web-buffer of prompt-buffer. Users will be confused for no good reason when starting Nyxt with the config below. Notice that emacs-mode won't be enabled in editor-buffers. It breaks the principle that @jmercouris raised above about managing the user's expectations.

(define-configuration prompt-buffer
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

(define-configuration web-buffer
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

(define-configuration nyxt/mode/editor:editor-buffer
  ((default-modes (pushnew 'nyxt/mode/editor:plaintext-editor-mode %slot-value%)))

(@aartaka) emacs-mode/vi-mode is a keyscheme-mode, and, by extension, a mode. Which means it only works in modable-buffer.

The observation above is sensible, and motivates commit 43585e5. An example of a buffer that is an input-buffer but isn't a modable-buffer is the status-buffer. Indeed, it makes little sense to add keyscheme modes, such as emacs-mode, for all input-buffers (though it's benign to do so). I.e. the first sexp is seperior than the second to achieve the goal of propagating the keyscheme.

(define-configuration (input-buffer modable-buffer)
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

(define-configuration input-buffer
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

[aartaka]

I'm not convinced by the argument that proposes separating setting the keyscheme for web-buffer and prompt-buffer. From the user's perspective, using technical jargon such as prompt buffer in the common-settings (a page where they may have just landed after installing Nyxt) is not wise.

This is a matter of language used in common-settings, not the matter of web-buffer/prompt-buffer separation. Language can be easily fixed, so this is not much of a hindrance.

From a technical perspective, the separation isn't exhaustive as well. Think of editor-buffer and note that it doesn't inherit from either web-buffer of prompt-buffer. Users will be confused for no good reason when starting Nyxt with the config below. Notice that emacs-mode won't be enabled in editor-buffers. It breaks the principle that @jmercouris raised above about managing the user's expectations.

Indeed, we've forgotten the editor-buffer. We might include it too, for the sake of completeness!

#| ... |#
(define-configuration buffer
  ((default-modes (pushnew 'nyxt/mode/editor:plaintext-editor-mode %slot-value%)))

Did you mean editor-buffer here? I'm confused about what idea you're trying to convey by this piece of code. Can you expand?

(@aartaka) emacs-mode/vi-mode is a keyscheme-mode, and, by extension, a mode. Which means it only works in modable-buffer.

The observation above is sensible, and motivates commit [...] I.e. the first sexp is seperior than the second to achieve the goal of propagating the keyscheme.

(define-configuration (input-buffer modable-buffer)
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

But wait, this enables Emacs mode for:

• input-buffers (including status-buffer),
• modable-buffers,
• and the intersection of these.

Booleand OR, in other words.

I guess that you meant is just the intersection (boolean AND). For that, you need to target a class that is an intersection of input-buffer and modable-buffer. Which web-buffer, prompt-buffer, editor-buffer, and panel-buffer (now that we talk of completeness) are.

EDIT: Broken quotation syntax.
EDIT: Boolean logic references.
EDIT: modable-buffer typo.

[aadcg]

Did you mean editor-buffer here? I'm confused about what idea you're trying to convey by this piece of code. Can you expand?

Yes, thanks. I've fixed it. I'm conveying that setting a keyscheme mode for web-buffers and prompt-buffers isn't exhaustive, as you note as well.

I guess that you meant is just the intersection (boolean AND).

Correct. I had the wrong idea about the semantics of define-configuration. I know understand how it handles the classes passed to it.

Which web-buffer, prompt-buffer, editor-buffer, and panel-buffer (now that we talk of completeness) are.

Today that is correct. We just forgot about editor-buffer, which shows that this is not robust.

While I agree that it's odd to add a keyscheme mode to an entity like status-buffer, in the sense that it's a no-op, it does nothing harmful. I still find the first configuration to be more robust than the second, in the snippet below:

(define-configuration input-buffer
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

(define-configuration (web-buffer prompt-buffer panel-buffer editor-buffer)
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

[aartaka]

Today that is correct. We just forgot about editor-buffer, which shows that this is not robust.

Not including the things we're not sure/aware/remembering about is more robust than including the things we're not sure/aware/remembering about. It's more dangerous to implicitly rely on all the entities in a set—including forgotten ones—than relying on a small yet safe subset of these. So not including forgotten editor-buffer is more robust than including the forgotten editor-buffer and half a dozen eldritch horrors lurking in the shadow of input-buffer umbrella.

While I agree that it's odd to add a keyscheme mode to an entity like status-buffer, in the sense that it's a no-op, it does nothing harmful.

Once we add one/N more input-buffer subclass for which keyscheme is not a no-op, we're at risk of breaking things the exact moment we add this subclass.

I still find the first configuration to be more robust than the second, in the snippet below:

(define-configuration input-buffer
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

(define-configuration (web-buffer prompt-buffer panel-buffer editor-buffer)
  ((default-modes (pushnew 'nyxt/mode/emacs:emacs-mode %slot-value%))))

I find the second one more robust, because:

• It clearly lists the classes it influences.
  • Which means that if we add other input-buffer subclasses, then it won't break in unexpected ways.
• It is easier to modify to toggle the keyscheme per class.
  • When one opens their config file, which one looks more intimidating—abstract input-buffer or obvious (given at least half an hour of Nyxt use) web-buffer etc.?

In the context of this PR—with most of the team agreeing that web-buffer/prompt-buffer/whatever difference should at least be highlighted, at most separated on the level of UI—the second form is superior too, because the mapping from UI to configuration is extremely clear. You configure keyscheme for web-buffer—you see the configuration form for it.

[aadcg]

I've pushed a version that seems to be a good middle ground between all opinions.

[aartaka]

Nut wait, that doesn't solve the problem of prompt-buffer/web-buffer discrepancy yet 😜 Maybe:

• An additional prompt buffer-related :nselect?
• Or at least a comment explaining that the configuration has this totality?

---

In any way, it would also be nice to split the configuration in two:

• web-buffer, panel-buffer, and editor-buffer.
• And prompt-buffer.

So that it's easier for one to edit the configuration.

---

Yet another detail: define-configuration can have a docstring, and it wouldn't hurt adding one here 😄

[aartaka]

The prompt-buffer vs. anything is the minimum actionable thing, everthing else is optional.

[aadcg]

As I've mentioned, my proposal wanted to be a good middle ground between all opinions. The goal of this PR is straightforward: fix common-settings is such a way that the expectations of new users are met. I don't think it is in our interest to rethink how many dropdowns set the keybindings or focus on other orthogonal topics right now.

We discussed how the goal of setting the keyscheme globally could be achieved, though no consensus was reached. At the end of the day, it's not the most important topic so I simply gave in.

The only (possibly) actionable seems to be:

at least a comment explaining that the configuration has this totality?

Well, if there's a single dropdown isn't it rather obvious? The point of auto configuration is that users set options via the UI. Black belts can craft their own configs. But we digress again from the focus of this PR.

---

@jmercouris @Ambrevar please advice on how to proceed.

[aartaka]

Well, if there's a single dropdown isn't it rather obvious?

Given the uniqueness of prompt buffer as an interface, it might be unclear whether it also respects this configuration.

The point of auto configuration is that users set options via the UI.

So they can also choose prompt buffer setting via UI.

Black belts can craft their own configs.

Making their (and first-time users transitioning though the Nyxt karate dans) work slightly easier is a net-positive change, especially given how little work it requires. And making Nyxt more approachable (to everyone) is a central goal, as I understood.

But we digress again from the focus of this PR.

Based on the linked issue (#3096), the proper fix is also disambiguating betwen prompt and web buffers (which was the initial confusion in the issue). So making the configuration more global may solve some of the problems (propagation of Emacs/VI bindings to both web and prompt buffer), but it consts us another problem ("why is web-buffer configuration doesn't influence prompt-buffer" i.e. disambiguation of the two).

[jmercouris]

Two problems.

1. This doesn't do what the user expects
2. The user wants more fine grained control

this PR solves point 1. Point 2 is a separate one, and probably not suited for Common Settings. A single user does not constitute a pattern. We shouldn't make the experience of everyone worse for the benefit of a single user. As this PR stands right now, it is ready.

[aartaka]

We shouldn't make the experience of everyone worse for the benefit of a single user.

Here's a set of user stories and their result for interface use with one or two <select>s:

| Situation\Interface                         | 1 <select> | 2 <select>s | Notes                       |
| Casual: Doesn't know what there is, DWIM    | ~          | ~           |                             |
| Casual/Hacky: Wants to configure everything | +          | -           | 1 vs. 2 clicks              |
| Hacky: Wants to configure web-buffer        | ~          | +           | Ambiguous vs. clear         |
| Hacky: Wants to configure prompt-buffer     | ~          | +           | Ditto                       |
| Hacky: Curious, looks through settings      | ~          | +           | Total vs. isolated settings |

Where:

• + means getting to the desired result easily and understanding what happened.
• ~ means getting confused or achieving the uncertain result.
• - means not achieving the result or having inconveniences.

Translating + to 1, ~ to 0, and - to -1, we get:

• 1 <select>: 1.
• 2 <selects>: 2.

In other words, the clarity that separate configs provide overweights the inconvenience of having to make an additional click. For both hacky users and casual ones, and for several user stories that are likely to happen.

[jmercouris]

Let's agree to disagree. Reminder: this is also not related to your main tasks.

[aartaka]

Reminder: this is also not related to your main tasks.

😰

[Ambrevar]

I agree (;p) that it is not a very big deal, and that using different bindings for different things can easily be done outside of Common Settings for now.

It's also not hard to add a second <select>, but this can be done later when the Common Settings are more polished, and feature and "advanced" section for instance.

[aadcg]

@jmercouris my understanding is that we should merge this fix. Please do it if that's the case, thanks.

[jmercouris]

OK, will do.

[aadcg]

@jmercouris merged so that the fix lands on the release today.

[jmercouris]

Sorry about that, thank you for merging.