textual icon indicating copy to clipboard operation
textual copied to clipboard

Input templates

Open amottola opened this issue 1 year ago • 15 comments

Issue: https://github.com/Textualize/textual/issues/4581. Sorry, couldn't wait for proper discussion, but of course this is open to changes. Please be gentle, this is my first pull request :blush:

Please review the following checklist.

  • [x] Docstrings on all new or modified functions / classes
  • [x] Updated documentation
  • [x] Updated CHANGELOG.md (where appropriate)

amottola avatar Jun 03 '24 14:06 amottola

Looks like a lot of work! Could you summarize the changes, and maybe add screenshots / video?

willmcgugan avatar Jun 03 '24 14:06 willmcgugan

In input.py I've added a _Template class that does most of the heavy lifting. As specified in the docs, a template mask assigns for each character to be allowed in the input field a regular expression based on the category of the mask character. See the docs updated by this PR (or Qt input masks since my implementation tries to follow the same rules, with the only exception being [ ] { } are not implemented - which should not be a problem since they are reserved in the Qt implementation anyway) for details on mask characters; for example N corresponds to the [0-9a-zA-Z] regex, meaning that if N is the 3rd character in the template, when cursor_position is 2 only an user input that matches that regex is allowed. We also have meta characters that allow for forced case conversions and to specify the blank character (again see the docs). The blank character is what appears - using the placeholder style - where a character can be entered by the user, unless `placeholder´ is specified - in such case the placeholder has priority and is displayed in place of the "empty" character. All characters in the template that are not mask or meta characters are considered separators: they are automatically inserted when the user reaches them while typing, and are automatically removed when the user deletes all the chars next to them. I think seeing this in action clarifies a lot of things... I've added some test cases and a snapshot test FWIW.

For an example, here you go:

from textual.app import App, ComposeResult
from textual.widgets import Input


class TemplateApp(App[None]):
    def compose(self) -> ComposeResult:
        yield Input(template=">NNNNN-NNNNN-NNNNN-NNNNN;_")
        yield Input(template="9999-99-99", placeholder="YYYY-MM-DD")


if __name__ == "__main__":
    app = TemplateApp()
    app.run()
Screenshot 2024-06-03 alle 19 19 28

The first thing to note is that the template also forces the maximum input size. As you can see, the placeholder for the first Input uses _ as it is specified as blank character (the default if unspecified is space). The second Input has an explicit placeholder that overrides any template blank character; if placeholder is smaller than the template mask, the blank character is used to fill missing entries.

Screenshot 2024-06-03 alle 19 27 57

Here we have entered ABCDE in the first Input, and since the next expected character is a separator, it is added automatically and the cursor advances next to it. Note that the template adds an implicit Validator and since the template specifies all N mask chars (which specifies a char matching regex [0-9a-zA-Z] is required), the input is not considered valid. In the second Input below a full date was inserted (by typing 2,0,2,4,1,2,3,1 - separators are automatically inserted) as since this satisfies all of the chars in the template, input is considered valid.

I don't know of a tool specifically tailored to make videos of a terminal; if you point me to one, I'd be more than happy to make a video that is worth more than thousand words.

amottola avatar Jun 03 '24 17:06 amottola

I've just reworked docs a bit to be more clear (I hope!) with an example of using templates to input a credit card number. Just run make docs-serve-offline and navigate to the Input widget docs.

amottola avatar Jun 05 '24 14:06 amottola

@amottola I've extracted the big if-elif block into a dictionary lookup, and added some missing type hints. Also added an xfail test for a case where "cursor word right" is failing.

darrenburns avatar Jun 11 '24 16:06 darrenburns

Nice catch about converting that big if-elif block to dict lookup. See my comment above about the failing test for action_cursor_right_word() and how I fixed it

amottola avatar Jun 11 '24 18:06 amottola

There's another issue that worries me: currently all Input attributes are compatible with the "template mode" enabled by setting the template, except for max_length; the template completely overrides it and makes it completely dependent on the template mask. How should we deal with this discrepancy? Should we use a watcher and raise an exception if max_length is changed while in template mode? Another possibility is just explicitly mentioning in the docs the fact that such an attribute will not be respected while in template mode. Suggestions are welcome!

amottola avatar Jun 11 '24 18:06 amottola

@darrenburns I've added a validate_value() to ensure if a value is set and it does not match current template (if set), an exception is raised. Not sure about this last change though as makes we wonder why the same approach is not used for the restrict parameter...

amottola avatar Jun 14 '24 09:06 amottola

I also think there might be a class with "suggesters" as they change the background/dim text if there's a matching suggestion.

It's making me wonder if this functionality could live as a subclass of Input which exposes only the compatible constructor parameters/methods.

darrenburns avatar Jun 17 '24 12:06 darrenburns

That's a thing that started to tick in the back of my head while creating this whole PR... So I propose to open up a second PR with a refactor that leaves Input alone and introduces a new class derived by it. Since template here is an abused term, I propose to name the new class MaskedInput, having the template parameter replaced with a mask one. We can then compare both PRs and choose the most appropriate one (but I suspect the second one will fit best). What do you think of this approach?

amottola avatar Jun 17 '24 13:06 amottola

That's a thing that started to tick in the back of my head while creating this whole PR... So I propose to open up a second PR with a refactor that leaves Input alone and introduces a new class derived by it. Since template here is an abused term, I propose to name the new class MaskedInput, having the template parameter replaced with a mask one. We can then compare both PRs and choose the most appropriate one (but I suspect the second one will fit best). What do you think of this approach?

I think that makes sense. @willmcgugan given how this interferes a bit with some of the existing functionality, do you think a more specialised MaskedInput subclass widget makes sense here?

darrenburns avatar Jun 20 '24 08:06 darrenburns

Sorry if I'm missing something, for there is quite a bit of code here.

First, this is explicitly modeled after Qt, which does enforce its license. That is, Qt is available under GPL and commercial licenses. The documentation language is under the FDL, which appears copied in the pull request. Some would argue that language documentation is not protectable, though it is a real argument. This can attach to the entire Textual tree and create lots of legal questions unless Qt Corporation explicit disclaims it. See https://doc.qt.io/qt-6/licensing.html for some background.

Second, one can imagine scenarios where validation decisions are different. For example, if validating a long code which is likely to pasted in instead of typed in, one would want to take the string, portion it up correctly, and color the errors. If the code is usually hand entered, one might choose to ignore non-conforming keystrokes. This means pasting "123 Main Street" in a five digit zip code field would show parts of the input in red, allowing the operator to know the wrong field is being pasted in a way the ignoring bad data and getting "123' would not. One size cannot fit all.

The standard solution allows parameters for the per key validation function and for either a function or regex for the full validation. That is, a logic like "when a key is typed, call to the validation function with the key and cursor position, and receive back the updated string". Similarly, allow a ValidatedInput which refuses to move the cursor out of the field when the text fails to validate. The standard solution would handle the Qt validation as well as other schemes.

merriam avatar Jul 08 '24 15:07 merriam

First of all, sorry for the late reply.

I understand your concerns regarding the Qt licensing issue; I have experience with their licensing policies so I get your fears there. But here I just copied the idea, there is absolutely no code sharing, not even remotely (this is an implementation written from scratch in Python based only on the final desired behavior). I copied parts of the documentation at first because I was lazy to write my own, but then I applied several modifications where we diverged and was appropriate to do so. I can further differentiate the docs so it is not copied anymore; then what is left is just the fact I mimicked the behavior... which I don't think is subject to any possible legal action. But I'm no lawyer so feel free to correct me if I'm wrong.

About your second concern, let me reinstate that this PR does not replace normal Input validation, rather it extends it: the usual (and custom) validators can still be added to the field for all the cases you describe and more. In truth, the whole template mask this PR adds is validated by an actual validator than is hidden and always added to the internal list of the Input validators. The whole idea of templates is to ease the validation of the most simple use cases like telephone numbers, dates, IP addresses, serial numbers, etc. For anything more complex you can either still use templates but with an additional validator(s) added to the field, either not use templates at all.

amottola avatar Jul 13 '24 14:07 amottola

Ouch. Copying doesn't work that way. Sorry, once you copy the documents, all other changes are derivative works. I admit to living with an IP lawyer and it has warped my senses away from common sense to what is actually litigated. You are creating a derivative work with the documentation, which requires a license. For the code, that's an open (and expensive) question. While common practice treats APIs and specifications as non-creative, the question is one for an IP lawyer. My rememberance is that Ashton-Tate (DBase) vs Borland (FoxBase) was lost because Ashton-Tate did not have clear license to DBase, not because the language specification was unworthy of copyright. The answer, as always, is "its a bit more complicated...."

merriam avatar Jul 14 '24 20:07 merriam

I love picture validation and would actually want to see constants for "CreditCard", "Phone-US", "Phone-DE", "Zip+9", etc. I feel that we have a lot incremental separate controls for the same concept.

Current, Input has:

  • type of integer, float, or text. Typing a letter into an input field means it is ignored. Even this basic view leads to people having requirements like "range" or "non-crazy unicode" that needs to build upon it.
  • restrict taking a regex. If the newly entered character fails to match the regex, the input is rejected. It is unclear if valid_empty overrides the regex.
  • validators, called at changed, substituted or blur, are functions taking the current input string with False meaning to ignore input.
  • max_length is the maxium length of input.

The order of checking is max_length, restrict, type, then validators, though that is not documented. Also, rejection from validators does not trigger the trigger bell while others do. These conditions all decide whether to throw out a change, and dead code in validation.py actually tries to generalize this. Validators are not run if the value of widget is assigned outside of a key press and this causesis_valid() to become stale.

There should be clear vocabulary differentiating Intermediate ("valid so far"), Complete ("valid field value"), and Accepted ("accepted value by logic of the form)". For example, "-" is an Intermediate number but not a Complete number. Similarly, a blank "ccv" might be OK, but cause submitting a form to block if a credit card number is entered, meaning the ccv is Complete but not Accepted. It would be useful to call are_all_Input_Complete(myContainer) when grabbing the form values, especially if there is business logic changing field values.

The Input Template feature appears just to be another input validator, though it is expected to change the cursor position and the input string. If we raise validators to a first class idea, we could easily end of up something like, yield Input(id=.., validators=[Max_Length(4), Type_Positive_Integer, Regex('[0-9,]+'), Template('9,999')].

I suggest:

  • the max_length, type, and restrict parameters document themselves as validators,
  • we adopt the input templates (after license issues) as another validator
  • add constants for some common validators, like "NonNegative", "Range", or "CreditCardNumber"
  • change is_valid or add is_complete to run validators before returning

merriam avatar Jul 14 '24 22:07 merriam

I see your point and your proposal is intriguing. But for Input Templates to work as normal input validators, the validator protocol would need to also accomodate for:

  • custom keyboard behavior: for templates like NNNN-NNNN-NNNN-NNNN, shortcuts to go to previous/next word takes you to previous/next group, even if group is empty (example: suppose your cursor is at the end of ____-____-ABC, hitting the previous word key combo twice would bring your cursor next to the first -). Note that the blank character can be anything, even ., so using the normal word skipping behavior does not apply.
  • custom rendering: Templates right now always display placeholder text even if current input value is not empty.

Adjusting the validators protocol to support these issues is a stretch to me, and not what validators were intended for. At least the second issue could be solved by using a wrapper class so one could write something like Input(placeholder=StickyPlaceholder('____-____-____-____')). But then this could conflict with Suggesters... and we potentially open another can of worms there.

amottola avatar Jul 16 '24 21:07 amottola

Closing - Implemented in #4783 and is now released.

darrenburns avatar Sep 30 '24 10:09 darrenburns