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

Change option 'previouspattern' to 'prevpattern'. #570

Merged
merged 2 commits into from Oct 2, 2016

Conversation

@lolilolicon
Copy link
Contributor

@lolilolicon lolilolicon commented Aug 6, 2016

For backward compatibility, 'previouspattern' remains valid.

The reason for this change: "prev" is the standard LINK TYPE, while
"previous" is an often supported (but incorrect) synonym. Therefore,
followDocumentRelationship("prev") is preferred.

Reference:
https://www.w3.org/TR/html4/types.html#type-links
https://www.w3.org/TR/html5/links.html#sequential-link-types
https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types

For backward compatibility, 'previouspattern' remains valid.

The reason for this change: "prev" is the standard LINK TYPE, while
"previous" is an often supported (but incorrect) synonym. Therefore,
followDocumentRelationship("prev") is preferred.

Reference:
https://www.w3.org/TR/html4/types.html#type-links
https://www.w3.org/TR/html5/links.html#sequential-link-types
https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types
@lolilolicon
Copy link
Contributor Author

@lolilolicon lolilolicon commented Aug 6, 2016

I'd like to bring up a related issue here.

It's very convenient to use [[ and ]] for pagination. Currently <link rel=...> always takes precedence over hints. It's cool but unfortunately creates a problem for this particular page, which contains such <link>s:

<link rel="next" href="cnx.org/contents/AgQDEnLI@6.20:XZe6d2Jr/What-Is-Sociology">
<link rel="prev" href="cnx.org/contents/AgQDEnLI@6.20:Z5TcGN3a/Preface">

Clearly, each of the href is incorrectly specified, which should have been prefixed with e.g. http:// or simply //. As a result, ]] on this page navigates to:

http://cnx.org/contents/AgQDEnLI@6.20:TrIRM88K@3/cnx.org/contents/AgQDEnLI@6.20:XZe6d2Jr/What-Is-Sociology

What do you propose to resolve this?

I've tried to use autocmd to fix or remove those <link>s, to no avail (not sure why).

Could we perhaps add an option which toggles following <link>s here?

EDIT: On this page, pressingf then ne actually goes to the next page. Perhaps not a big enough hassle to do anything about. Let move on, I guess :)

@@ -1648,7 +1648,7 @@ const Buffer = Module("buffer", {

mappings.add(myModes, ["[["],
"Follow the link labeled 'prev', 'previous' or '<' if it exists",
function (count) { buffer.followDocumentRelationship("previous"); },
function (count) { buffer.followDocumentRelationship("prev"); },

This comment has been minimized.

@gkatsev

gkatsev Aug 6, 2016
Member

this doesn't actually just look for link tags with a rel of previous.
It actually looks at the option for previouspattern and then looks at link, a, and whatever is in hinttags option and see whether it patches the previouspattern pattern.

This comment has been minimized.

@lolilolicon

lolilolicon Aug 6, 2016
Author Contributor

Yes, I understand that. With this change, [[, instead of <link rel="previous", follows <link rel="prev", everything else remains the same.

@lolilolicon
Copy link
Contributor Author

@lolilolicon lolilolicon commented Aug 7, 2016

See a real world example why prev instead of previous here, where we have:

<link href="https://www.jwz.org/blog/2016/08/cymothoa-exigua-represent/" title="*Cymothoa exigua* represent" rel="prev">

Since currently vimperator looks for rel="previous", [[ on this page does not follow the link above. Instead, it follows hint, which happens to match

<a href="https://www.jwz.org/blog/2016/07/">« Jul</a>
@maxauthority
Copy link
Member

@maxauthority maxauthority commented Oct 2, 2016

I think the additions are worthwhile although I am not really sure about the name change of the option, but well, both names are valid, so why not.

@maxauthority maxauthority merged commit c045269 into vimperator:master Oct 2, 2016
@lolilolicon
Copy link
Contributor Author

@lolilolicon lolilolicon commented Oct 2, 2016

Well for one thing, now these two line up nicely 😉

prevpattern
nextpattern
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.