-
Notifications
You must be signed in to change notification settings - Fork 345
feat: add @match directive support #2003
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
base: master
Are you sure you want to change the base?
feat: add @match directive support #2003
Conversation
- Implemented URL pattern matching functionality for @match directives - Added support for domains with hyphens in pattern matching - Fixed regex pattern to properly parse domains like aflt-systems.ru - Updated urlMatchPattern function to handle subdomain wildcards correctly
9184c5d to
5aaecb9
Compare
| import {ownRoot} from '@/js/urls'; | ||
| import {stringAsRegExpStr, tryRegExp, tryURL} from '@/js/util'; | ||
| import * as colorScheme from '../color-scheme'; | ||
| import { styleCodeEmpty } from "@/js/sections-util"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please undo all cosmetic changes (formatting/quotes/indents and so on) as it makes next to impossible to use git features like blame or history grep or diff as they become too "noisy" in almost any git client.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tophf Is it better now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost. There's still prefs.js and manage-newui.css.
c730e3e to
292f48a
Compare
- Revert formatting changes (quotes, spacing, line breaks) in all modified files - Preserve functional changes for @match directive support - Keep original code style to avoid git history noise - Affected files: matcher.js, usercss-manager.js, install-usercss/index.js, meta-parser.js, sections-util.js, usercss-compiler.js, render-favs.js, render.js
292f48a to
19ecb24
Compare
tophf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I found some more trivial issues in the code, it can wait until we decide on the high-level design in #2002.
| const {sourceCode: code, [UCD]: {vars, preprocessor, match}} = style; | ||
|
|
||
| // Pass @match data to compiler | ||
| const varsWithMatch = vars |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK this is the same as const varsWithMatch = {...vars, _usercssData: {match}}
| replaceChildren($('.external-link'), makeExternalLink()); | ||
| getAppliesTo().then(list => | ||
| replaceChildren($('.applies-to'), list.map(s => $create('li', s)))); | ||
| replaceChildren($('.applies-to'), list.map(s => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- We shouldn't show favicons if the user disables the option to show favicons in the manager (usually for privacy).
- We should reuse the existing code that shows icons for all targets, not just
@match. - Since this is not exactly trivial, maybe it should be done in a separate PR
|
|
||
| const PREPROCESSORS = new Set(['default', 'uso', 'stylus', 'less']); | ||
|
|
||
| // Custom parser for @match directive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done in https://github.com/openstyles/usercss-meta
| import {deepCopy, deepEqual, isCssDarkScheme, makePropertyPopProxy} from './util'; | ||
| import {onStorageChanged} from './util-webext'; | ||
| import './msg-init'; // installs direct `API` handler | ||
| import { k_busy } from "@/js/consts"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting/syntax shouldn't be changed (in the entire file).
| const res = extractSections({code}); | ||
|
|
||
| // Process @match directives from metadata | ||
| if (vars && vars._usercssData && vars._usercssData.match) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the modern syntax: vars?._usercssData?.match
| padding: 0 0.5em; | ||
| display: flex; | ||
| flex-wrap: wrap; | ||
| flex-wrap: nowrap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Changes in essential things like entry styling must be convincingly justified/explained.
| overflow: hidden; | ||
| max-height: calc(var(--num-targets) * 18px); | ||
| width: 100%; | ||
| display: flex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
| } | ||
| .target { | ||
| display: block; | ||
| display: inline-block; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
| line-height: var(--target-height); | ||
| width: 0; | ||
| min-width: 100%; | ||
| width: auto; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
| } | ||
| } | ||
| element.dataset.type = type; | ||
| element.dataset.targetValue = targetValue; // Store original value for favicon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the necessity to have separate displayValue and target.dataset.targetValue. Looks like the renderFavs function's regexp can be changed to extract the domain from target.textContent
Overview
Adds support for
@matchdirective in usercss files.Closes #2002
What's Added
@matchdirective support for URL pattern matching@matchdirectives@matchcompatibility@matchpatterns in style management and installationUsage
Styles with
@matchdirectives will now only apply to the specified domains.UI Changes
@matchdomainsFiles Changed
src/js/meta-parser.js- Added @match parsersrc/background/style-manager/matcher.js- Enhanced matchingsrc/background/usercss-manager.js- Updated usercss handlingsrc/js/usercss-compiler.js- Added @match compilationsrc/js/sections-util.js- Better sections processingsrc/js/prefs.js- Enhanced preferencessrc/install-usercss/index.js- Added @match support and favicon displaysrc/manage/files - UI updates and @match display