Skip to content

Conversation

@Thom1729
Copy link
Member

For #136. Includes #160.

The basic moving parts are:

  • The generic GenericListInputHandler and GenericTextInputHandler classes.
  • The decorator, which enables the generator-based syntax.

These, are, in principle orthogonal, though this initial implementation isn't quite. With a slightly different implementation, you could yield any arbitrary CommandInputHandler rather than just these two special generic handlers.

Usage example (a minor adaptation of the example from the issue):

import sublime
import sublime_plugin

from sublime_lib import fancy_input, GenericListInputHandler, ResourcePath, new_view

class InputTestCommand(sublime_plugin.WindowCommand):
    def run(self, path):
        content = sublime.load_resource(path)
        new_view(
            self.window,
            name=path,
            content=content,
            syntax=sublime.find_syntax_for_file(path),
            read_only=True,
            scratch=True,
        )

    @fancy_input
    def input(self, args):
        print('input')
        path = ResourcePath('Packages')

        while not path.exists():
            path_str = yield GenericListInputHandler(
                name='path',
                items=[
                    str(child)
                    for child in path.children()
                ]
            )
            path = ResourcePath(path_str)

Notes:

  • The fancy_input name could use some work.
  • Obviously we're going to want unit tests.
  • Some attributes like placeholder aren't present yet.
  • I'm not sure how we would/could incorporate cancel().

Looking for input.

__all__ = ['fancy_input', 'GenericListInputHandler', 'GenericTextInputHandler']


def fancy_input(
Copy link
Member

Choose a reason for hiding this comment

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

A more descriptive suggestion: "input_generator"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Works for me; renamed.

Copy link
Member

Choose a reason for hiding this comment

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

FYI, imports don't work anymore if the exported function has the same name as a module. The module needs to be renamed. I picked input_handlers.py locally.

from functools import wraps

FancyInputHandler = Union['GenericListInputHandler', 'GenericTextInputHandler']
GeneratorType = Generator[FancyInputHandler, object, None]
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to resolve the object type through a type variable of the input handler's items param.

self,
*,
name: str,
items: list
Copy link
Member

Choose a reason for hiding this comment

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

Other params we should look into supporting (and decide whether they are too advanced or not):

  • placeholder
  • initial_text
  • initial_selection
  • validate
  • preview
  • description
  • confirm (equivalent to the sent value into the generator)
  • cancel

For cancel we could throw an exception to the generator.

@FichteFoll
Copy link
Member

For the more complex case, i.e. a recursive input handler or similar, I find this implementation to be very flexible. However, it doesn't satisfy the initial criteria of #136, whose goal is to reduce boiler-plate as much as possible, notably when you don't want to implement input yourself. I do think that a solution could easily be built upon this.

With a slightly different implementation, you could yield any arbitrary CommandInputHandler rather than just these two special generic handlers.

I believe that trying to preserve the type signature of the input handlers (via type variables) would still be possible using protocols, so unless it would unnecessarily complicate our implementation, striving for naive input handler compatibility should be our goal.

@Thom1729
Copy link
Member Author

An issue I've run into is that if the user presses backspace, things get weird. I'm not sure if there's a good way to handle this (we can't just rewind the generator). I think that the backspace issue is a hard blocker. Hopefully there's a good way around it.

Other than the above, we could definitely implement a more #136-like interface on top of this. If the backspace issue proves fatal, we should be able to implement that sort of interface in a different way; the backspace issue shouldn't prevent that.

(I wish we could just access the input handler UI directly.)

@FichteFoll
Copy link
Member

When the user presses backspace, the event listener's cancel method is invoked. We can't really do anything with this information from the library itself, so the caller will need to handle this case by catching our exception. Alternatively, we could send a 2-tuple into the generator, where one value represents the cancelled state and/or the previously yielded input handler, if that somehow works out?

Will need to experient with this myself.

@FichteFoll
Copy link
Member

I did some experiments with the example from OP, also with raising an exception for cancel. There are a few issues:

  1. We can't differentiate between backspace and escape (i.e. just pop the input handler stack or abort completely).
  2. When throwing an exception into the generator and the exception is not handled, the generator terminates but ST still happily displays the previous input handler. For the example, this then leads to an unexpected argument being passed to the command, since no new input handler is pushed in next_input. Ironically, the example Just Works without considering cancel because of how it overrides its state.
    Also, this scales horribly for multiple cancelations.
  3. ST reuses the previous input handler instance automatically. We can still intercept the following next_input call, but any code inside the generator between the two yields will not be re-run.

Honestly, this seems like a dead end. The fact that input handlers can be unwinded doesn't translate to an imperative generator.


I was able to come up with a pretty neat multi-level resource browser, but that turned out to be too complex to embed into a generic interface like we're looking for. I'll include it anyway.

Example for a complex input handler chain
import sublime
import sublime_plugin

from sublime_lib import ResourcePath, new_view


class TestCommand(sublime_plugin.WindowCommand):
    def run(self, path):
        content = sublime.load_resource(path)
        new_view(
            self.window,
            name=path,
            content=content,
            syntax=sublime.find_syntax_for_file(path),
            read_only=True,
            scratch=True,
        )

    def input(self, args):
        path = ResourcePath(args.get('path', 'Packages'))
        if not path.exists():
            return ResourcePathInputHandler('path', path)


class ResourcePathInputHandler(sublime_plugin.ListInputHandler):
    _selected = None  # type: ResourcePath

    def __init__(self, name: str, path: ResourcePath):
        self._name = name
        self._path = path

    def name(self) -> str:
        return self._name

    def list_items(self):
        return [
            sublime.ListInputItem(
                text=path.name + ('/' if not path.exists() else ''),
                value=str(path),
            )
            for path in self._path.children()
        ]

    def confirm(self, value: str) -> None:
        self._selected = ResourcePath(value)

    def description(self, value: str, text: str) -> str:
        return text.rstrip('/')

    def next_input(self, args):
        if not self._selected.exists():
            return ResourcePathInputHandler(self._name, self._selected)

@Thom1729
Copy link
Member Author

Yeah, I came to a similar conclusion. Since we can't rewind or duplicate a generator, I don't think there's any sensible way to handle backspaces within this paradigm.

@Thom1729 Thom1729 closed this Apr 12, 2021
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.

3 participants