Skip to content

Conversation

@Thom1729
Copy link
Member

Will be useful for #136 and more.

Copy link
Member

@FichteFoll FichteFoll left a comment

Choose a reason for hiding this comment

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

I didn't do a detailed review; just proceed with this.

@Thom1729 Thom1729 merged commit 0d741d5 into master Apr 11, 2021
@Thom1729 Thom1729 deleted the sublime-plugin-stub branch April 11, 2021 13:38
@FichteFoll FichteFoll added this to the 1.6.0 milestone Apr 11, 2021
Copy link

@gwenzek gwenzek left a comment

Choose a reason for hiding this comment

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

I think I spotted on issue for this PR.
In my InputHandler preview, validate and confirm receives only a string.

But https://github.com/jfcherng-sublime/ST-API-stubs/blob/master/sublime_plugin.pyi#L649 agrees with you.

def next_input(self, args: dict) -> Optional[CommandInputHandler]: ...
def placeholder(self) -> str: ...
def initial_text(self) -> str: ...
def preview(self, arg: dict) -> Union[str, sublime.Html]: ...
Copy link

Choose a reason for hiding this comment

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

I think this should be

def preview(self, arg: str)
def validate(self, arg: str)
def confirm(self, arg: str)

Also note that confirm can receive an event. Not sure how to handle this.
We could add def confirm_(self, arg: str, event: sublime.Event)

Copy link
Member

Choose a reason for hiding this comment

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

I haven't verified this right now, but from the documentation, the arg received should be the value of a ListInputHandler's list item, which may be any serializable, meaning ListInputHandler must be a generic class.

TextInputHandler would then inherit CommandInputHandler[str].

This means that ListInputItem needs to be generic as well, but we don't have that at all currently since it's ST4 only.

Copy link

Choose a reason for hiding this comment

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

Gotcha. I only tried on TextInputHandler.
What's the plan for Python 3.8 and ST4? Is there a specific branch I should look at?

class TextCommand(Command):
view: sublime.View

def run(self, edit: sublime.Edit) -> None: ...
Copy link

Choose a reason for hiding this comment

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

This is one is tricky. TextCommand can accepts more argument.
But subclassing run(self, edit: sublime.Edit, **args: Any) by run(self, edit: sublime.Edit, my_cmd_argument: str) isn't allowed anyway.

Copy link

Choose a reason for hiding this comment

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

I think just having run: Callable[..., None] would be more inline of mypy philosophy of "no false positive"

@Thom1729
Copy link
Member Author

@gwenzek I've addressed those issues in the linked PR.

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.

4 participants