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

Load prefs.json from profile-dir if --profile-dir is specified at launch #10114

Merged
merged 3 commits into from Apr 2, 2016

Conversation

@matthewbentley
Copy link
Contributor

matthewbentley commented Mar 22, 2016

In response to #10098
Tries to load prefs.json from the profile-dir and merge them into the preferences if --profile-dir is specified at launch. The profile-dir preferences take precedence over the default preferences, but command line preferences still take precedence over everything.

Also adds some tests for prefs.rs. These rely on the contents of resources/prefs.json (at least test_get_set_reset_extend() does), so they may need to be re-worked a bit.


This change is Reviewable

@highfive
Copy link

highfive commented Mar 22, 2016

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon.

@matthewbentley matthewbentley force-pushed the matthewbentley:master branch 3 times, most recently from 50d9d9b to aa48a40 Mar 22, 2016
@paulrouget
Copy link
Contributor

paulrouget commented Apr 1, 2016

follow up: #10338

extend_prefs(prefs);
}
} else {
println!("Error opening prefs.json from profile_dir");

This comment has been minimized.

@Manishearth

Manishearth Apr 1, 2016

Member

this should be to stderr.

@Manishearth
Copy link
Member

Manishearth commented Apr 1, 2016

Looks good to me overall, minor nit

@matthewbentley matthewbentley force-pushed the matthewbentley:master branch from aa48a40 to 8e3ce60 Apr 1, 2016
@matthewbentley
Copy link
Contributor Author

matthewbentley commented Apr 2, 2016

I updated it to write to stderr. I'm not sure if I did it right; I just googled and used the first example.

@Manishearth
Copy link
Member

Manishearth commented Apr 2, 2016

Looks good. Do you want to squash any commits?

@matthewbentley
Copy link
Contributor Author

matthewbentley commented Apr 2, 2016

Should I? I could probably squash the first 2 together and second two together.

@Manishearth
Copy link
Member

Manishearth commented Apr 2, 2016

That sounds good.

--profile-dir on launch)

Use T: Read rather than File, so that read_prefs_from_file can be tested
Also, spelling fix in opts.rs

Fix order of imports to satisfy the lint
Fix split line in test
Fix multi-line string
@matthewbentley matthewbentley force-pushed the matthewbentley:master branch from 8e3ce60 to 324b4d1 Apr 2, 2016
@matthewbentley
Copy link
Contributor Author

matthewbentley commented Apr 2, 2016

Squash'd

@Manishearth
Copy link
Member

Manishearth commented Apr 2, 2016

@bors-servo r+

thanks!

@bors-servo
Copy link
Contributor

bors-servo commented Apr 2, 2016

📌 Commit 324b4d1 has been approved by Manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Apr 2, 2016

Testing commit 324b4d1 with merge 51c5e60...

bors-servo added a commit that referenced this pull request Apr 2, 2016
Load prefs.json from profile-dir if --profile-dir is specified at launch

In response to #10098
Tries to load `prefs.json` from the profile-dir and merge them into the preferences if `--profile-dir` is specified at launch.  The profile-dir preferences take precedence over the default preferences, but command line preferences still take precedence over everything.

Also adds some tests for `prefs.rs`.  These rely on the contents of `resources/prefs.json` (at least `test_get_set_reset_extend()` does), so they may need to be re-worked a bit.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10114)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 2, 2016

The build was interrupted to prioritize another pull request.

@bors-servo
Copy link
Contributor

bors-servo commented Apr 2, 2016

Testing commit 324b4d1 with merge ea24389...

bors-servo added a commit that referenced this pull request Apr 2, 2016
Load prefs.json from profile-dir if --profile-dir is specified at launch

In response to #10098
Tries to load `prefs.json` from the profile-dir and merge them into the preferences if `--profile-dir` is specified at launch.  The profile-dir preferences take precedence over the default preferences, but command line preferences still take precedence over everything.

Also adds some tests for `prefs.rs`.  These rely on the contents of `resources/prefs.json` (at least `test_get_set_reset_extend()` does), so they may need to be re-worked a bit.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10114)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Apr 2, 2016

@bors-servo bors-servo merged commit 324b4d1 into servo:master Apr 2, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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

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