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

Umi suggestion filtering #93

Merged
merged 3 commits into from
Dec 26, 2022
Merged

Conversation

ctwrs
Copy link
Contributor

@ctwrs ctwrs commented Dec 24, 2022

This is implementing an initial version of tag filtering. The goal is to get suggestions that take previous tags into account. This will make Umi prompting viable for widespread use. Right now debugging prompts it a bit tedious because we have to run the prompt to get debug info about what keywords get matched.

I still have to find a good solution for optional tags, so that it will match all other tags no matter where they are defined and only limit the selection to what's filtered through neg and pos parts.

Screen.Recording.2022-12-24.at.01.34.47.mov

Code quality and naming feedback appreciated. Wrote it late at night and I'm surprised it even works.

@DominikDoom
Copy link
Owner

DominikDoom commented Dec 24, 2022

Looking pretty good, just for clarification: This only shows completions for tag combinations still possible with what comes before, correct?

Other than that, the console.log on line 730 can be removed or commented out, I just forgot to take that out after I finished testing the original implementation. It's a bit too spammy for normal use. The rest of the logs probably as well if you don't need them further.

I don't have time to try it out myself right now, but I will trust you that it doesn't break unrelated functionality.

@ctwrs
Copy link
Contributor Author

ctwrs commented Dec 24, 2022 via email

@ctwrs ctwrs changed the title WIP: Umi suggestion filtering Umi suggestion filtering Dec 26, 2022
@ctwrs
Copy link
Contributor Author

ctwrs commented Dec 26, 2022

OK, had to rewrite it a bit because the initial implementation was only optimised for a single umi tag in a prompt. Moreover, the filtering logic depends on which tag the cursor is focused at and this further complicated things. I feel it's pretty solid now. Please double-check me before merging if you find a minute because I'm far past my bedtime.

@DominikDoom
Copy link
Owner

Functionality wise, it seems good to me, normal completion also doesn't seem impacted, so all good to go.

The only issue I noticed is that editing negative tags sometimes doesn't show completion if later tags have already restricted the tags further. E.g., <[female][fullbody][--western][chinese]> will not show completion when going back to edit --western, since western has already been removed from the candidates by the later chinese. Technically it should still show since negative could be anything not explicitly required by the chinese, but that's hard to do since retroactive changes to front tags would obviously impact later ones. Since this format is especially supposed to work with sequential completion, I don't think it's a big issue, so I'll still merge it now. Maybe we can think of something later, but that's not a priority.

Other than that there's only one console.log left that fills the console with the matches object, but I'll remove that myself after merging. There's still another one left in unrelated code, so I'll just do them both together.

@DominikDoom DominikDoom merged commit ceba611 into DominikDoom:main Dec 26, 2022
@ctwrs
Copy link
Contributor Author

ctwrs commented Dec 26, 2022

Yeah, retroactive editing is a bit clunky but it shouldn't be a big issue (I hope). We'll see what feedback people will leave.

Thanks for merging it :).

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.

2 participants