Add dropdown to find a highlighted word in am-morphs when studying

xofm31 commented 6 months ago

https://github.com/mortii/anki-morphs/issues/187

When studying, be able to highlight text on the card and have the pulldown menu to browse cards that have that text in am-unknowns.

What is the new behavior?

The desired behavior is that when the option is selected, the card browser will come into focus with a query to find cards with the highlighted text.

There are at least 2 bugs in the code right now:

It does the search as soon as you right click to get the dropdown. I don't know how to use the trigger.
The query ought to have quotes around it, in case there are spaces in either the quoted text or the field name. I tried to do this by adding quotes in the f-string, doing it without an f-string, and using re.escape. I really thought re.escape should work, but it only escaped the hyphen in "am-unknowns" and didn't include the quotes.

What kind of changes does this PR introduce?

This shouldn't change any other functionality.

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Checklist:

[ ] My code successfully passes pre-commit
[ ] I have commented my code, particularly in hard-to-understand areas
[ ] I am willing to help create documentation/guides for the changes made in this PR
[ ] This PR can be rebased onto main without conflicts (create a backup branch before attempting a rebase)
[ ] I have squashed my commits into sensible portions

mortii commented 6 months ago

There are at least 2 bugs in the code right now:

It does the search as soon as you right click to get the dropdown. I don't know how to use the trigger.

The query ought to have quotes around it, in case there are spaces in either the quoted text or the field name. I tried to do this by adding quotes in the f-string, doing it without an f-string, and using re.escape. I really thought re.escape should work, but it only escaped the hyphen in "am-unknowns" and didn't include the quotes.

These are very fixable, I'll take a look in a little bit.

mortii commented 6 months ago

Does this query string give you the results you want?

query = f'"{ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}:{selected_text}"'

I found that the surrounding quotes disappear when the text doesn't have any whitespaces in it, but I believe that is an intentional auto-correct since the results are the same.

The reason it runs when you right click is because you do all the logic in the action function, but that function is only for setting up the action, not running it. Instead you need to connect a separate function that runs on trigger, e.g: https://github.com/mortii/anki-morphs/blob/1e22414ac84066959b4151f54471bf24a4c406ed/ankimorphs/__init__.py#L469

xofm31 commented 6 months ago

Does this query string give you the results you want?

Yes, I think that's what I started with. I didn't realize it would only put the quotes when needed. It looks right to me.

Instead you need to connect a separate function that runs on trigger

I had tried to do a trigger calling a function before, but without the lambda. I added the lambda, and now it is working. Thanks!

I pushed a new commit.

mortii commented 6 months ago

Nice, that's some beautiful code :clap:

How do you feel about the "Browse in am-unknowns" label? It took me a a second to decipher what it meant, whereas something like "Find cards with this am-unknowns" would eliminate that, at least for me. Do you have a preference? My suggestion might be too long, so idk :shrug:

xofm31 commented 6 months ago

I don't feel strongly about anything. What about:

action = QAction(f"Find cards with \"{ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}:{selected_text}\"", menu)

Or something like that, that actually shows the selected text?

Also, once I completely move over to Ankimorphs, it's my plan to set my ankimorphs_globals.EXTRA_FIELD_UNKNOWNS to "Word", to be compatible with my other cards (I still need to do a bit of investigation that this will work the way I want). Are you planning to make renaming the fields an option, or should I just plan on editing my personal copy of ankimorphs_globals.py?

mortii commented 6 months ago

I don't feel strongly about anything. What about:

action = QAction(f"Find cards with \"{ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}:{selected_text}\"", menu)

Or something like that, that actually shows the selected text?

That's pretty genius, It didn't cross my mind that the label could be dynamic. The Mark as Name feature should also use that.

With that addition, it could just be:

f'Browse "{ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}:{selected_text}"'

to make it shorter, or do you prefer the "Find cards with" first?

Are you planning to make renaming the fields an option, or should I just plan on editing my personal copy of ankimorphs_globals.py?

It was optional in the early alpha, but it turned out to be a complete disaster because people (myself included) were careless and important fields ended up being overwritten... so no, I don't plan on changing that.

xofm31 commented 6 months ago

to make it shorter, or do you prefer the "Find cards with" first?

Just "Browse" as you suggest is better.

It was optional in the early alpha, but it turned out to be a complete disaster because people (myself included) were careless and important fields ended up being overwritten... so no, I don't plan on changing that.

Got it. As long as you consistently use the fields as defined in ankimorphs_globals, I can set it my own fork, and if I overwrite something, it will be my own fault.

mortii commented 6 months ago

I'm going to tweak this slightly and then I'll merge it into main

mortii / anki-morphs