Refactor WebLinkMatcher.ts and add additional. by Calamitish · Pull Request #1924 · VocaDB/vocadb

Calamitish · 2025-04-24T21:23:42Z

ExtLinks.css:

Replace all double quotes with single quotes for consistency.
Add trailing slashes to all URLs for consistency.
Clean up usage of background-image vs background.
Unify http and https checks. Before this, some icons would only match with http URLs and some only with https URLs.
Make bracket usage consistent.
Remove or combine duplicate entries.
Removed a dead comment.
Re-order ExtLinks.css by logo image name alphabetically.
- With exception for those those that have a logo in a different repository, move those to the top.
Rename fasic and thbwiki to Fasic and THBWiki to match actual filenames. naming convention for files could be a discussion point for a future PR.
Add icon matching with links that did have icons, but were not being used.
Add additional icon matching with new icons (see: Add additional website logos. ExtIcons#7)
Icon matching via ^ match or * match has been left mostly the same, could still be a discussion point for a future PR.

WebLinkMatcher.ts

Add additonal Weblink matchers relating to Add additional website logos. ExtIcons#7

# ExtLinks.css: - Re-order ExtLinks.css by logo image name alphabetically. - - With exception for those those that have a logo in a different repository, move those to the top. - Clean up usage of background-image vs background. - Fix icon matching with links that only matched to http:// to also work with https:// - Add icon matching with links that did have icons, but were not being used. - Add additional icon matching with new icons (see: VocaDB/ExtIcons#7) - Icon matching via ^ match or * match has been left mostly the same, could still be a discussion point. - Clean up styling. # WebLinkMatcher.ts - Add additonal Weblink matchers relating to VocaDB/ExtIcons#7

Susko3

To me, this PR is impossible to review. It's possible that you accidentally changed a link category or dropped some icon. It would be way easier to check that if this PR was split into separate commits that do one of the steps you mention in the description.

Also, every line in the diff is marked as changed, even if it remained the same. Have the line-endings accidentally been changed? Maybe make the line-ending change a separate commit, or don't do it. https://docs.github.com/en/get-started/git-basics/configuring-git-to-handle-line-endings

This reverts commit 4efdaa5.

Calamitish · 2025-04-26T16:27:04Z

every line in the diff is marked as changed

Seems like the project is operating with LF line endings which automatically got converted to CRLF on my end.
Will revert and separate in different commits.

That last one's still going to be a pain to review just by nature of sorting all of the ExtLinks alphabetically.

Some matchers would only match with http or https URLs.

…t where neccesary.

- bsky - dojin-music

Calamitish · 2025-04-26T18:21:53Z

+	background-image: url('/Content/ExtIcons/ototoy.png');
+}
+
+a.extLink[href^='http(s?)://potune.jp/']


I have also removed the incorrect a.extLink[href^='http://ototoy.jp/'] here.

See: VocaDB/ExtIcons#7 - ah-soft - ai-voice - bandlab - dreamtonics - genius - github - marshmallow-qa - musixmatch - project sekai fandom wiki - reddit - voicevox - voisona

Calamitish · 2025-04-26T18:51:13Z

PR updated and everything is now split into separate commits.
Let me know if I've missed something.

Something to note:
It seems that the project either inconsistently uses LF and CRLF line-endings or only uses LF.
(It's very hard for me to tell, in all honesty)
It's probably a good idea to either outline this in the wiki entry for windows development environment or the project readme.
It's currently easy to miss this difference for any contributors.
Might also be a good idea to sweep the entire project and double check that all files consistently use the same line-endings.

Susko3

I've checked that the individual commits make changer per their description. Everything else seems fine, apart from the following:

- Re-add accidentally removed link matchers. - Fix brace.

Susko3 · 2025-04-26T21:15:23Z

+}
+
+/* image outside of ExtIcons folder because it is also used elsewhere*/
+a.extLink[href^='http(s?)://soundcloud.com/'],


Have you tested this? Regex doesn't seem to be supported in CSS attribute selectors.

I suggest you either
a) go back to separate http:// and https://
b) change the matcher to be [href*='://soundcloud.com/'] (anywhere in url, but including :// to avoid false-positives)

Went with b as to not have unnecessary duplication.

Susko3 · 2025-04-26T21:49:41Z

Whoops, the review went wrongly, just search for "?" and you'll see the problems.

Calamitish · 2025-04-26T22:00:39Z

Got them!
Here's to writing code when you're tired :D
I probably typo'd those originally so they didn't get caught be the replace.

Susko3

Seems to be good now!

FinnRG

Thank you both for your work on this.

Susko3 suggested changes Apr 26, 2025

View reviewed changes

Calamitish added 2 commits April 26, 2025 18:22

Revert "Refactor WebLinkMatcher.ts and add additional."

cb3889e

This reverts commit 4efdaa5.

Add additional linkmatchers.

283ea99

Calamitish added 12 commits April 26, 2025 18:27

Replace all double with single quotes.

bc81fd0

Add trailing slash to all URLs.

8e7196d

Replace all instances of background: with background-image:

002ffd5

Unify http and https checks.

8a01a68

Some matchers would only match with http or https URLs.

Make bracket usage consistent.

1c5c6f5

Remove or combine duplicate entries.

6dd89f6

Split Dwango and Animelo to use correct image.

413291a

Remove dead comment.

841e76b

Move matchers with images not in ExtIcons to the top and add a commen…

19dc496

…t where neccesary.

Sort all icon matchers alphabetically by png filename.

b151f30

Adjust fasic and thbwiki to match filename.

1f4c21e

Add icon matchers that have images but no matchers.

832059c

- bsky - dojin-music

Calamitish commented Apr 26, 2025

View reviewed changes

Calamitish added 3 commits April 26, 2025 20:25

Add additional icon matchers.

70dcadd

See: VocaDB/ExtIcons#7 - ah-soft - ai-voice - bandlab - dreamtonics - genius - github - marshmallow-qa - musixmatch - project sekai fandom wiki - reddit - voicevox - voisona

Fix missed unified http/https check.

0246310

Add back trailing slashes.

521e724

Susko3 suggested changes Apr 26, 2025

View reviewed changes

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css Outdated

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css Outdated

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css Outdated

Process feedback.

672537d

- Re-add accidentally removed link matchers. - Fix brace.

Susko3 reviewed Apr 26, 2025

View reviewed changes

Remove regex from link matchers.

acc0a21

Susko3 suggested changes Apr 26, 2025

View reviewed changes

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css

Fix malformed link matchers.

dde86d8

Susko3 reviewed Apr 26, 2025

View reviewed changes

Comment thread VocaDbWeb/Scripts/styles/ExtLinks.css Outdated

Set archive matcher last so it always takes precedence.

6da47fe

Susko3 approved these changes Apr 26, 2025

View reviewed changes

FinnRG approved these changes Apr 27, 2025

View reviewed changes

FinnRG merged commit 22955f5 into VocaDB:main Apr 27, 2025
1 check passed

Uh oh!

Conversation

Calamitish commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ExtLinks.css:

WebLinkMatcher.ts

Uh oh!

Susko3 left a comment

Choose a reason for hiding this comment

Uh oh!

Calamitish commented Apr 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Calamitish Apr 26, 2025

Choose a reason for hiding this comment

Uh oh!

Calamitish commented Apr 26, 2025

Uh oh!

Susko3 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Susko3 Apr 26, 2025

Choose a reason for hiding this comment

Uh oh!

Calamitish Apr 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Susko3 commented Apr 26, 2025

Uh oh!

Calamitish commented Apr 26, 2025

Uh oh!

Uh oh!

Susko3 left a comment

Choose a reason for hiding this comment

Uh oh!

FinnRG left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Calamitish commented Apr 24, 2025 •

edited

Loading

Calamitish commented Apr 26, 2025 •

edited

Loading