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

Implements goto prev/next page feature #97

Merged
merged 1 commit into from
Sep 27, 2018

Conversation

isundaylee
Copy link
Contributor

For now I implemented a very naive algorithm that looks for with
matching text. It would probably be better to somehow reuse the logic in
the hint link discovery, but it is actually subtly different here. In
hint link discovery, we do a DFS from the root document, and stop
recursing whenever we identify a link. But here we actually want to
visit every child, since the text could potentially be on one of the
child elements instead.

Text matching patterns for next/previous page links can be configured as
nextpagetextpatterns and prevpagetextpatterns options in sVimrc.

Default settings tested on Google, and GitHub issue page.

For now I implemented a very naive algorithm that looks for <a> with
matching text. It would probably be better to somehow reuse the logic in
the hint link discovery, but it is actually subtly different here.  In
hint link discovery, we do a DFS from the root document, and stop
recursing whenever we identify a link. But here we actually want to
visit every child, since the text could potentially be on one of the
child elements instead.

Text matching patterns for next/previous page links can be configured as
`nextpagetextpatterns` and `prevpagetextpatterns` options in sVimrc.

Default settings tested on Google, and GitHub issue page.
@abrookins
Copy link
Collaborator

Hi, Jiahao! Thanks much for your PR. I will review soon -- this is a feature I've wanted from sVim for a long time. :)

@abrookins
Copy link
Collaborator

abrookins commented Sep 5, 2018

@isundaylee Thanks so much for your time writing up this PR! I think the direction I'd like to see the next/previous button go is more along the lines of how cvim does it: https://github.com/1995eaton/chromium-vim/blob/fe60f5fa18e5f6ec379494c346127a221307aff7/content_scripts/hints.js#L27

If you look at how Hints.matchPatterns works over there (it's MIT-licensed, so feel free to use whatever code you want -- if you're interested in continuing to work on this feature!), you'll see that cvim cheats a bit in a good way. That is, the plugin stores a few CSS targets for commonly-used sites, and tries to match one to the current URL first. Only then, when it can't find a match, does is compile a regex object and search the available links on the page. Something like that would be best.

@isundaylee
Copy link
Contributor Author

isundaylee commented Sep 5, 2018 via email

@abrookins
Copy link
Collaborator

I'm going to merge this as-is so we get this feature into the 1.0.7 release, and we (or you, if you're interested!) can iterate on it if needed. Also, thanks so much for looking into this feature! It was #1 on my list of things I wished sVim did.

@abrookins abrookins merged commit cdfc39e into flippidippi:master Sep 27, 2018
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