chores: Broaden the NND MyList link matcher #1896
Conversation
Match both - https://www.nicovideo.jp/mylist/000 (old URL style) and - https://www.nicovideo.jp/user/000/mylist/000 (new URL style)
| // matching both | ||
| // https://www.nicovideo.jp/mylist/000 (old URL style) and | ||
| // https://www.nicovideo.jp/user/000/mylist/000 (new URL style) | ||
| url: '/mylist/', |
There was a problem hiding this comment.
I would prefer to have two entries in WebLinkMatcher.ts for the new and the old style rather than including this one matcher without a host
There was a problem hiding this comment.
I initially did that, but then I realized that the one with the host would be redundant as the short one would match a superset; but I also understand the argument that it's stylistically better
(well, actually, I put in nicovideo.jp/user/\d+/mylist/, before realizing that these are not regular expressions; then I reduced it to /mylist/ and then replaced the matcher for the old style entirely…)
There was a problem hiding this comment.
I'm really sorry for the late reply. My main argument for two separate entries is to still keep the host (https://www.nicovideo.jp) in the matcher to keep URLs who just happen to include /mylist/ from matching with this matcher
There was a problem hiding this comment.
My understanding is that
https://www.nicovideo.jp/mylist/000can be matched withnicovideo.jp/mylist/(current behavior), buthttps://www.nicovideo.jp/user/000/mylist/000must be matched with the host-less/mylist/(proposed edit), and cannot have a host (ideallynicovideo\.jp/user/\d+/mylist/if represented in regex) due to the simplistic use ofindexOf(not regex) that cannot handle the necessary wildcard for the intervening/user/000/, andnicovideo.jp/mylist/would then be a redundant test to/mylist/.
This *does* allow "false positives" for other hosts, but I am not sure if this would happen in practice.
Match both