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

Command palette #3058

Merged
merged 230 commits into from
Sep 11, 2023
Merged

Command palette #3058

merged 230 commits into from
Sep 11, 2023

Conversation

davep
Copy link
Contributor

@davep davep commented Aug 4, 2023

Implements #3021.

Key features of the PR:

  • Adds a "command palette" (AKA minibuffer if you're an Emacs fan) that pops up over a Textual application.
  • Adds a method of providing command sources, all of which will be searched.
  • Runs the code associated with the command once a choice has been made.
  • Hits from sources are gathered up async, so the completions can keep coming in as the user interacts with the system.

Current state of work in a simple test application, where there's a command source of lines of text, where a selected line will cause the text to be shown in a Textual notification. The command source is coded to pretend to be a very slow source, with noticeable pauses between each command been provided.

Screen.Recording.2023-08-29.at.10.44.11.mov

The matching in the command provider is done using Textual's (as yet undocumented) builtin "fuzzy matcher". The code for the command provided used here is:

    async def search(self, query: str) -> CommandMatches:
        """A request to hunt for commands relevant to the given user input.

        Args:
            query: The user input to be matched.
        """
        print(f"begin search(\"{query}\")")
        try:
            # Get a Textual fuzzy matcher.
            matcher = self.matcher(query)
            # For each line in the data source...
            for candidate in self.DATA:
                # Sleep some random amount of time, just so we can pretend
                # to be a slow source.
                await sleep(random() / 10)
                # If the match is above zero...
                if matcher.match(candidate):
                    # ...create a hit object and pass it back up to the
                    # command palette for use there.
                    yield CommandSourceHit(
                        # The match score -- used for sorting.
                        matcher.match(candidate),
                        # The Rich Text object for showing the command and
                        # how it matched.
                        Text.assemble(
                            Text.from_markup("[italic green]notify('[/]"),
                            matcher.highlight(candidate),
                            Text.from_markup("[italic green]')[/]")
                        ),
                        # The code to run this if this match is picked by
                        # the user.
                        partial(self.screen.notify, candidate),
                        # The original text of the command itself.
                        candidate,
                        # Some optional help text for the command; here used
                        # to help show easy access to key information about
                        # the environment.
                        "Show the selected text as a notification\n"
                        f"I think the current screen is {self.screen!r}\n"
                        f"I think the focused widget is {self.focused!r}\n"
                        f"Match score: {matcher.match(candidate):0.5f}"
                    )
        except CancelledError:
            print(f"cancelled search(\"{query}\")")
        finally:
            print(f"end search(\"{query}\")")

where self.DATA is simply a list of lines of text.

A slightly more sensible example, this time hooked up with a command source that wraps the main Textual application actions:

Screen.Recording.2023-08-29.at.10.46.51.mov

Still to do (not an exhaustive list):

  • Improve and fix the fuzzy matcher.1
  • Add support to the fuzzy matcher for being given styles for highlighting.
  • Add a component class to help control the styling of matched text in the hits.
  • Look at a good sort order for command hits.
  • Related to the above, consider support for OptionList for inserting new options in specific locations in the list.
  • Add support for using the command palette in an example app or two (the code browser looks like a prime candidate).
  • Finally decide how the command palette will be called upon by the user, if it should be enabled by default, and how configurable that should be by the developer.
  • Add global enable/disable flag on App
  • Add a reference command source for obvious commands on App.
  • Unit tests.
  • Snapshot tests.
  • Main documentation.
  • Likely a full entry in the guide.2
  • Perhaps also a HOWTO on how to write your own command source.3
  • Test on Windows; especially the choice of default key binding.
  • Document the default key binding somewhere, so developers know how this is initially invoked.4
  • Try a short delay before the loading indicator appears. Suggest 500ms, to prevent flicker.
  • Remove the border that separates the input from the results.
  • Try adding a 🔎 emoji before the input text, which aligns with the results.
  • Have the highlight in the command list extend as far as possible left and right.
  • Add appropriate entries to the ChangeLog.
  • Update the "Released in 0.??.0" text to the release version.
  • Double-check the issues with Ctrl+Space on Windows, relating to pasting from the clipboard (see comment way down below).

Footnotes

  1. Actually, aside from some tweaks to the interface, I've left it as-is for the moment. I do wonder if we can improve the scoring -- perhaps scoring earlier matches over later and the like -- but I think it's fine for the moment.

  2. The more I think on this, the more I think it makes more sense that, initially at least, this is documented as part of the API (which it is).

  3. I think this should come later if it's decided it's needed.

  4. Going to be added by @willmcgugan before going into main; likely a main docs and blog combo.

davep added 30 commits July 27, 2023 15:19
This isn't the interface. Nowhere near. But this helps kick off visualising
how it will all work.
Just to help things stand out for the moment. At some point I think I'll
allow passing in custom styles, which will come from component classes or
something. For now though this makes it easier to see what's going on.
These were there when I was first testing the layout. They're not needed any
more.
At the moment it does nothing more than grab a new UUID, but this gives us
scope for throwing in some sort of callout to the providers to let them know
we're done.
Not like this, but kinda like this. Just experimenting at the moment, hence
the random sleeps in the core of the generator (to sort of fake a slow
background source).
It's not going to end up quite like this, but this gets it going.
Doesn't seem to make sense to have a LoadingIndicator constantly running in
the background when it isn't needed.
I don't really see much need for this, now that development of this is well
under way. And even if we do want to expose this, I think we need to allow
setting it on the class, not on the instance.
It's becoming clear that we do want to allow people to import from this
file, so it's time to drop the underscore.
There's one typing error that's been with me for weeks now, and nothing I
did seemed to get to the heart of it. Finally, I think it's dawned on me.
Raising NotImplemented from the abstract base implementation confuses the
type checker as it's not seeing any sort of yield going on. This... this
solves it.

I'm not 100% sure this is the correct thing to do, advice online seems
patchy at best and the couple of things I've seen that do seem to address
this sort of situation seem to introduce other typing issues (a bare yield
being the main suggestion, which won't work as then it'll be yielding the
wrong type).

Gonna sit on this for now and see how I feel about it, or see if I can find
something relevant to this.
The asend back into the search routing was always showing a typing mismatch,
but I couldn't quite see what was going on; what made it even more confusing
was the code was working fine.

It looks like keeping hold of the "routine", and keeping that distinct from
the iterator of the results, is the trick here. It all still works *and* the
typing works out.
src/textual/command_palette.py Outdated Show resolved Hide resolved
@davep davep requested a review from willmcgugan August 30, 2023 14:12
@davep
Copy link
Contributor Author

davep commented Sep 1, 2023

The choice of default key binding might be questionable after all: while it has been shown to work fine on macOS and Windows, I've just noticed that, in Windows 11 under Parallels at least, if I go to paste something into a Textual application from the clipboard it causes the command palette to appear first and then the text being pasted is pasted into that -- this would suggest that the paste operation is causing the same sequence to be passed in as that generated by ctrl+space.

We've recently changed the way the command list is cleared down when the
search term is modified, thus removing a source of "flashing" as the user
types; this pretty much involves *not* clearing down the previous hits until
the first new hit comes in. This is fine in all situations expect where the
last search was a "no matches" search.

In that situation the next search stats out saying "no matches". That's
correct, that's the result of the previous search, but in this case it's
unhelpful and potentially confusing. So this commit checks if that's the
state of the command list up front and clears that option from the list.
While it's kinda cool... it's not really very helpful if you're doing things
via textual-web; all you're going to do is start to use storage on the host
machine, not the client machine (unless they're the same thing, of course).
@davep davep merged commit 7ca5dd6 into Textualize:main Sep 11, 2023
22 checks passed
@davep davep deleted the M-x branch September 11, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command Palette
2 participants