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

Change return type of password_mut to Option<&mut Option<String>> #169

Closed
wants to merge 2 commits into from

Conversation

@KiChjang
Copy link
Member

KiChjang commented Feb 13, 2016

This is necessary because by default, if the URL ':' delimiter was not found in the URL, then password_mut() would return None, causing a later mutation to the password field in RelativeSchemeData impossible.

Review on Reviewable

@KiChjang
Copy link
Member Author

KiChjang commented Feb 13, 2016

@KiChjang KiChjang force-pushed the KiChjang:password-mut branch from f6fad4d to afa4a43 Feb 13, 2016
@SimonSapin
Copy link
Member

SimonSapin commented Feb 13, 2016

This is sensible, but I have an in-progress rewrite that makes it obsolete: https://github.com/servo/rust-url/compare/1.0 (The password and other components will not be independent Strings anymore. *_mut methods will be replaced with setters, though I’m not certain of the exact API yet.)

So making a breaking release if another one is coming soon may not be worth it. In the mean time, as discussed on IRC, you can use the Url::relative_scheme_data_mut() method and access the RelativeSchemeData::password field directly.

@KiChjang
Copy link
Member Author

KiChjang commented Feb 13, 2016

Ah ok, the rewrite makes more sense then.

@KiChjang KiChjang closed this Feb 13, 2016
@KiChjang KiChjang deleted the KiChjang:password-mut branch Feb 13, 2016
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

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