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

Python 3 support and search by name instead of full text search config option, allowing full search with zim: prefix #22

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

shanness
Copy link

@shanness shanness commented May 7, 2022

Fixes #14 #15 and #13

@sojusnik
Copy link

sojusnik commented May 8, 2022

@shanness where have you been all these years? :) Can't wait to test it out, but I'm not sure how to install it on Ubuntu. Can you give a simple instruction?

Oh, I see that you have your own repo https://github.com/shanness/zimsearch with your tweaks. Will go through the installation recommendations and report back.

@sojusnik
Copy link

sojusnik commented May 8, 2022

Installed the plugin with your tweaks!

That's what I did:

cd ~/.local/share/zim/plugins
git clone https://github.com/shanness/zimsearch.git
cd zimsearch
sudo pip install .

The first thing that stood out is that your plugin works only when Zim is already open. That's somehow confusing, because you normally would expect it to work even when Zim isn't running, at least this was the case with the previous version. I've already had the case that Zim was closed and I started to type the page title in Gnome's shell search, but it didn't appear, because Zim was closed.

Then I tried the full text search feature with zim:testing expecting to find testing in my notes, but it only created a new page called testing.

Maybe it would be more intuitive to use zim:testing for a full text search and create a new note, when the normal search, that is looking for a page title, isn't finding results, like described here?

Happens ob Ubuntu 21.10 with Zim 0.74.3-SNAPSHOT-t202204171730.

@shanness
Copy link
Author

Hi @sojusnik, it's working for me. If I do zim:testing, I get new page for about 20 seconds then the search results. I have about 900 page, perhaps you didn't wait long enough or have much more data to search? I also tested that it offer's a new page if nothing found (for both zim:blah and just blah).

Basically my HD goes crazy with zim:testing, so perhaps just wait till your CPU/IO spike goes down?

image

Added tip about how to do full search when only pages selected.
@shanness
Copy link
Author

BTW, this isn't my plugin, I've just patched it. And also did you configure it to search only names by default (overridden with zim: which I've added a note in a later commit to the below option)

image

@sojusnik
Copy link

Hi @sojusnik, it's working for me. If I do zim:testing, I get new page for about 20 seconds then the search results. I have about 900 page, perhaps you didn't wait long enough or have much more data to search? I also tested that it offer's a new page if nothing found (for both zim:blah and just blah).

It indeed does work … if I wait for about 10 seconds. Maybe showing an indicator that a full text search is performing wouldn't hurt, to avoid misunderstandings for new users.

BTW, this isn't my plugin, I've just patched it. And also did you configure it to search only names by default (overridden with zim: which I've added a note in a later commit to the below option)

Yeah. Because of your tweak, this great plugin is now usable again, but this issue still remains though:

The first thing that stood out is that your plugin works only when Zim is already open. That's somehow confusing, because you normally would expect it to work even when Zim isn't running, at least this was the case with the previous version. I've already had the case that Zim was closed and I started to type the page title in Gnome's shell search, but it didn't appear, because Zim was closed.

Nevertheless, thanks for updating it!

@shanness
Copy link
Author

shanness commented Jun 4, 2022

Hey @sojusnik ,

It indeed does work … if I wait for about 10 seconds.
Cool :)
Maybe showing an indicator that a full text search is performing wouldn't hurt, to avoid misunderstandings for new users.

I've never written a search provider. But if you have any idea how I'd do that, happy to give it a go (I did a bit of a search and couldn't find anything). Have you ever seen another search provider show an indicator like that (I might be able to hack it from their source).

Yeah. Because of your tweak, this great plugin is now usable again, but this issue still remains though:
The first thing that stood out is that your plugin works only when Zim is already open. That's somehow confusing, because you normally would expect it to work even when Zim isn't running, at least this was the case with the previous version.

Hmm, really? I didn't make any mods I don't think which should have changed that. I'm just really wrapping the search in
"Name: *search* Name: *terms*"
So no idea how that could have made it change if you needed zim running. You sure it worked in the version I forked?

I actually used a pile of code already written to migrate to Python 3. Maybe one of the other users patches broke that?
vantu5z/zimsearch@master...shanness:master#diff-7224c877cdb976eedc00f00cadca439d24b8672c5a5475fdd8a0ad9c07b58466L185

Nevertheless, thanks for updating it!

No worries :)

@sojusnik
Copy link

I've never written a search provider. But if you have any idea how I'd do that, happy to give it a go (I did a bit of a search and couldn't find anything). Have you ever seen another search provider show an indicator like that (I might be able to hack it from their source).

Unfortunately, no. Don't know how to code either.

So no idea how that could have made it change if you needed zim running. You sure it worked in the version I forked?

I'm sure it worked without having Zim to run in the background in the previous release, before @achadwick did his tweaks and added the search functionality.

I actually used a pile of code already written to migrate to Python 3. Maybe one of the other users patches broke that?

Can't help you with that, sorry, this code looks like gibberish to me.

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.

Search for title, not content
3 participants