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

Changed empty-string behavior of text inputs #1954

Merged
merged 3 commits into from
Jul 20, 2023

Conversation

RunDevelopment
Copy link
Member

Changes:

  • The default value of min_length is now 0.
  • Text inputs will now strictly enforce length requirements.
  • Text inputs will now interpret an empty input as "no input". Except if allow_empty_string is true, then they will treat an empty input as the empty string.
    • Only a few text inputs allow the empty string. It's not just the one in the Text node. I tested all text nodes and allowed empty strings where the empty string is very useful (e.g. Text Replace with replacement="" to remove text).

Open question: current save files.
Current save files can have the empty string in text inputs that previously already had min_length=0 and that now do not allow the empty string. This creates a weird situation where the empty string is a valid input as long as you don't touch it (then it will be removed).
How should we deal with this? Should we just live with this weirdness? Should we reset non-allowed empty string input to "no value" and break some chains? Should we make a migration?

@joeyballentine
Copy link
Member

This creates a weird situation where the empty string is a valid input as long as you don't touch it (then it will be removed).

This sounds fine to me. It won't break existing chains but it'll go to the new behavior as soon as the chain is changed. That sounds unobtrusive enough

@RunDevelopment
Copy link
Member Author

I added a migration

@joeyballentine joeyballentine merged commit 2f002d2 into chaiNNer-org:main Jul 20, 2023
13 checks passed
@RunDevelopment RunDevelopment deleted the text-input-change branch July 20, 2023 17:31
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.

None yet

2 participants