mirror of
https://github.com/servo/servo.git
synced 2025-09-22 04:40:09 +01:00
Auto merge of #22923 - peterjoel:issue_8539_prefs_refactor, r=jdm
#8539 Config preferences backend restructure <!-- Please describe your changes on the following line: --> A procedural macro for generating static structures for use as the backend for config preferences, as well a mapping from string names to accessors. Preferences can be accessed and updated via a map-like interface with `String` keys, and now also via a convenience macro: `get_pref!(path.to.pref)`. Various `serde`-compatible field attributes are also supported, including renaming the string keys. This could be useful when changing the backend structure without breaking existing usages. I have added the choice to use `i64` as well as `f64` for storing numbers. As it turns out, none of the existing preferences used non-integer values. Setting a floating point value from a command-line argument requires a decimal point in order to parse correctly. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #8539 <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because ___ <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> ----- I have a few outstanding problems or questions: 1. I am unable to get rid of this warning: ``` warning: unnecessary path disambiguator --> components/config/prefs.rs:130:51 | 130 | accessor_type = crate::pref_util::Accessor::<Prefs, crate::pref_util::PrefValue>, | ^^ try removing `::` ``` See: https://stackoverflow.com/questions/54710797/how-to-disable-unnecessary-path-disambiguator-warning 2. Several of the preferences in use were not represented in `prefs.json`. Some of these may be in error, but it is hard to tell. For example `js.offthread_compilation.enabled` vs `js.ion.offthread_compilation.enabled` could be different preferences or could be intended to have the same value. 3. Previously, several pieces of code provided default values when accessing a preference that may not be present. For example: ```Rust let DBL_CLICK_TIMEOUT = Duration::from_millis( PREFS .get("dom.document.dblclick_timeout") .as_u64() .unwrap_or(300), ); ``` Fallback values don't really make sense now and I've added these defaults to `prefs.json`. Does this sound right? 4. I have kept `PrefValue::Missing`, though it doesn't seem very useful any more. An error might be more appropriate now for an incorrect preference key. I've kept this partly because [`webdriver_server` uses it](https://github.com/servo/servo/blob/master/components/webdriver_server/lib.rs#L224). <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22923) <!-- Reviewable:end -->
This commit is contained in:
commit
5ec725488f
53 changed files with 1748 additions and 680 deletions
|
@ -9,7 +9,7 @@ use selectors::attr::*;
|
|||
use selectors::parser::*;
|
||||
use servo_arc::Arc;
|
||||
use servo_atoms::Atom;
|
||||
use servo_config::prefs::{PrefValue, PREFS};
|
||||
use servo_config::set_pref;
|
||||
use servo_url::ServoUrl;
|
||||
use std::borrow::ToOwned;
|
||||
use std::cell::RefCell;
|
||||
|
@ -339,7 +339,7 @@ impl ParseErrorReporter for TestingErrorReporter {
|
|||
|
||||
#[test]
|
||||
fn test_report_error_stylesheet() {
|
||||
PREFS.set("layout.viewport.enabled", PrefValue::Boolean(true));
|
||||
set_pref!(layout.viewport.enabled, true);
|
||||
let css = r"
|
||||
div {
|
||||
background-color: red;
|
||||
|
|
|
@ -6,7 +6,7 @@ use cssparser::{Parser, ParserInput};
|
|||
use euclid::TypedScale;
|
||||
use euclid::TypedSize2D;
|
||||
use servo_arc::Arc;
|
||||
use servo_config::prefs::{PrefValue, PREFS};
|
||||
use servo_config::set_pref;
|
||||
use servo_url::ServoUrl;
|
||||
use style::context::QuirksMode;
|
||||
use style::media_queries::{Device, MediaList, MediaType};
|
||||
|
@ -45,7 +45,7 @@ fn test_viewport_rule<F>(css: &str, device: &Device, callback: F)
|
|||
where
|
||||
F: Fn(&Vec<ViewportDescriptorDeclaration>, &str),
|
||||
{
|
||||
PREFS.set("layout.viewport.enabled", PrefValue::Boolean(true));
|
||||
set_pref!(layout.viewport.enabled, true);
|
||||
let stylesheet = stylesheet!(css, Author);
|
||||
let mut rule_count = 0;
|
||||
stylesheet.effective_viewport_rules(&device, &stylesheet.shared_lock.read(), |rule| {
|
||||
|
@ -445,7 +445,7 @@ fn cascading_within_viewport_rule() {
|
|||
|
||||
#[test]
|
||||
fn multiple_stylesheets_cascading() {
|
||||
PREFS.set("layout.viewport.enabled", PrefValue::Boolean(true));
|
||||
set_pref!(layout.viewport.enabled, true);
|
||||
let device = Device::new(
|
||||
MediaType::screen(),
|
||||
TypedSize2D::new(800., 600.),
|
||||
|
|
|
@ -1,11 +1 @@
|
|||
prefs: ["layout.flex.enabled:true",
|
||||
"layout.flex-flow.enabled:true",
|
||||
"layout.flex-direction.enabled:true",
|
||||
"layout.flex-wrap.enabled:true",
|
||||
"layout.flex-grow.enabled:true",
|
||||
"layout.flex-shrink.enabled:true",
|
||||
"layout.justify-content.enabled:true",
|
||||
"layout.align-items.enabled:true",
|
||||
"layout.align-self.enabled:true",
|
||||
"layout.align-content.enabled:true",
|
||||
"layout.columns.enabled:true"]
|
||||
prefs: ["layout.columns.enabled:true"]
|
||||
|
|
|
@ -1,2 +0,0 @@
|
|||
prefs: ["layout.flex.enabled:true",
|
||||
"layout.flex-direction.enabled:true"]
|
Loading…
Add table
Add a link
Reference in a new issue