fonol / anki-search-inside-add-card

An add-on providing full-text-search and PDF reading functionality to Anki's Add card dialog
https://ankiweb.net/shared/info/1781298089
GNU Affero General Public License v3.0
179 stars 24 forks source link

Cleaned up general import #194

Closed aviral-batra closed 3 years ago

aviral-batra commented 3 years ago

1) Converted functions returning list of files and directories to generators so the programme can handle scanning large directories 2) Added list widget to show which files were found 3) Removed progress bar (to declutter the dialog, and now that there is the list of files found, it is redundant) 4) Cleaned up designer output (e.g. removed buddies that were not necessary) 5) Corrected error which ignored all files that did not start with '.' as opposed to those that did start with '.' when the checkbox to ignore all of those directories is selected

aviral-batra commented 3 years ago

I have a question: what is the benefit of using returnPressed as opposed to textEdited for updating the list when typing in the line edit?

fonol commented 3 years ago

Nice, thanks.

I have a question: what is the benefit of using returnPressed as opposed to textEdited for updating the list when typing in the line edit?

Short answer: when I begin typing, and reach C:/, Anki freezes. 😄 Maybe the searching should happen in a QRunnable or something like that, because it freezes the main UI thread otherwise.

aviral-batra commented 3 years ago

Ah, just tried that, looks like I overlooked it.

Maybe the searching should happen in a QRunnable or something like that, because it freezes the main UI thread otherwise.

Yeah, I'll look into it.

aviral-batra commented 3 years ago

@fonol Do you think it might be better to just have a limit after which it automatically triggers the 'show top level directories' checkbox/only shows the first few levels of directories and subdirectories instead of running it on a separate thread? If the search becomes thread blocking it's likely that it's trying to add hundreds of items to the list widget, which is impractical to choose from.

Edited addition: The actual importing itself might have to be put in a QRunnable, I just realised, as that will be an even bigger thread blocking task if the user has a large list of files.

fonol commented 3 years ago

I think the code overhead of using a QThread or QRunnable is not very large, so in any case, it might be the best decision. If you set a time limit, after which the search only returns the top-level results, I think you'd have to set it low to still have a smooth typing experience. But then there is the case when what when you actually want to search a larger directory, and you'd have to disable the implicit limit somehow. Maybe the least complicated setup is:

aviral-batra commented 3 years ago

I did both: I'll send the pull request in now. I'm not too sure how to abort the search/ kill the thread though.