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.

Sign up for GitHub

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

Jump to bottom

Add as_bytes() for FromUtf8Error. #40290

Merged
merged 6 commits into from
Apr 18, 2017
Merged

Add as_bytes() for FromUtf8Error. #40290

merged 6 commits into from
Apr 18, 2017

Conversation

3Hren
Copy link
Contributor

@3Hren 3Hren commented Mar 6, 2017

This change allows to obtain an underlying invalid UTF-8 bytes without FromUtf8Error destruction. Such method may be useful for example in a library that attempts to save both valid and invalid UTF-8 strings in some struct and to be able to provide immutable access to it without destruction.

Personally without this change I ended with Result<String, (Vec<u8>, Utf8Error), which almost copies the functionality of FromUtf8Error, but allows immutable view access.

@rust-highfive
Copy link
Collaborator

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@3Hren
Copy link
Contributor Author

3Hren commented Mar 6, 2017 •
edited
Loading

Should I have to add a stability attribute? If yes - which one?

@bluss
Copy link
Member

bluss commented Mar 6, 2017

cc @SimonSapin

@alexcrichton
Copy link
Member

Seems reasonable to me, thanks @3Hren!

Also yeah in terms of library stability this'll start out with an #[unstable] tag and then it'll go through the normal stabilization process. You don't have to create a tracking issue just yet, though. We can postpone that to just before the PR is merged.

@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 6, 2017
@aturon
Copy link
Member

aturon commented Mar 28, 2017

Ping @3Hren, this is just waiting on the unstable attribute.

@3Hren 3Hren mentioned this pull request Mar 29, 2017
Closed
@3Hren
Copy link
Contributor Author

3Hren commented Mar 29, 2017

@aturon sorry, completely forgot about this PR :(.
Fixed!

@aturon
Copy link
Member

aturon commented Mar 29, 2017

@3Hren Thanks! Note the Travis failure, which is legit.

@3Hren
Copy link
Contributor Author

3Hren commented Apr 8, 2017

Oh, looks like there is a new linter.

Unstable feature 'from_utf8_error_as_bytes' needs to have a section in The Unstable Book

How can I fix this?

@aturon
Copy link
Member

aturon commented Apr 10, 2017

cc @rust-lang/docs, can you spell out the procedure here?

@frewsxcv
Copy link
Member

Add a new file in this directory corresponding to name of the unstable feature and then just link to it from the summary file.

I plan to add a little README in https://github.com/rust-lang/rust/blob/master/src/doc/usntable-book/ soon describing this.

@frewsxcv
Copy link
Member

@3Hren You'll also need to link to that new page from the summary file.

@3Hren
Copy link
Contributor Author

3Hren commented Apr 11, 2017 •
edited
Loading

Finally, random travis errors non-associated with the PR :)

@Mark-Simulacrum
Copy link
Member

Can you remove the merge commit and force push? That'll both clean up the history and restart Travis.

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2017
@3Hren
Add as_bytes() for FromUtf8Error.
5a09d7c 
This change allows to obtain an underlying invalid UTF-8 bytes
without `FromUtf8Error` destruction. Such method may be useful
for example in a library that attempts to save both valid and
invalid UTF-8 strings in some struct and to be able to provide
immutable access to it without destruction.
@3Hren
fix: add feature to doc tests
4cb9de4
@3Hren
refactor: add reason string
0eecd51
@3Hren
docs: track newly introduced unstable feature
4cea3f7
@3Hren
fix: add missing link in SUMMARY.md
3ae20de
@3Hren
Copy link
Contributor Author

3Hren commented Apr 14, 2017

Done.

@Mark-Simulacrum
Copy link
Member

Error looks spurious.

[00:38:46] Rustbook (x86_64-unknown-linux-gnu) - unstable-book
[00:38:46] An error occured:
[00:38:46] Read-only file system (os error 30)
[00:38:46] 
[00:38:46] 
[00:38:46] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools/x86_64-unknown-linux-gnu/release/rustbook" "build" "/checkout/src/doc/unstable-book" "-d" "/checkout/obj/build/x86_64-unknown-linux-gnu/doc/unstable-book"
[00:38:46] expected success, got: exit code: 101
[00:38:46] 
[00:38:46] 
[00:38:46] Build completed unsuccessfully in 0:02:22
[00:38:46] Makefile:24: recipe for target 'all' failed
[00:38:46] make: *** [all] Error 1

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 14, 2017
frewsxcv
frewsxcv reviewed Apr 17, 2017
View reviewed changes
src/doc/unstable-book/src/SUMMARY.md Outdated
@@ -78,6 +78,8 @@
- [fmt_internals](fmt-internals.md)
- [fn_traits](fn-traits.md)
- [fnbox](fnbox.md)
- [field_init_shorthand](field-init-shorthand.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line supposed to be here? This is causing the 'read only' fail that Travis is encountering

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it landed here due to bad rebase.

@frewsxcv frewsxcv mentioned this pull request Apr 17, 2017
Closed
@3Hren
Copy link
Contributor Author

3Hren commented Apr 17, 2017

Passed!

@carols10cents carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2017
@arielb1
Copy link
Contributor

arielb1 commented Apr 18, 2017

Friendly ping for @aturon.

@aturon
Copy link
Member

aturon commented Apr 18, 2017

Huzzah!

@bors: r+

@bors
Copy link
Contributor

bors commented Apr 18, 2017

📌 Commit bbdf190 has been approved by aturon

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 18, 2017
@frewsxcv
Rollup merge of rust-lang#40290 - 3Hren:master, r=aturon
353bdb3 
Add `as_bytes()` for `FromUtf8Error`.

This change allows to obtain an underlying invalid UTF-8 bytes without `FromUtf8Error` destruction. Such method may be useful for example in a library that attempts to save both valid and invalid UTF-8 strings in some struct and to be able to provide immutable access to it without destruction.

Personally without this change I ended with `Result<String, (Vec<u8>, Utf8Error)`, which almost copies the functionality of `FromUtf8Error`, but allows immutable view access.
@frewsxcv frewsxcv mentioned this pull request Apr 18, 2017
Merged
bors added a commit that referenced this pull request Apr 18, 2017
@bors
Auto merge of #41373 - frewsxcv:rollup, r=frewsxcv
9f2abad 
Rollup of 6 pull requests

- Successful merges: #40290, #41353, #41356, #41360, #41361, #41364
- Failed merges:
@bors bors merged commit bbdf190 into rust-lang:master Apr 18, 2017
@frewsxcv frewsxcv mentioned this pull request Apr 27, 2017
Merged
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 27, 2017
@frewsxcv
Bump mdbook dep to pick up new 'create missing' toggle feature.
a517a96 
This will avoid obscure Travis CI error messages:

* rust-lang#40290 (comment)

Original mdbook issue:

* https://github.com/azerupi/mdBook/issues/253

mdbook PR:

* https://github.com/azerupi/mdBook/pull/254
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 27, 2017
@frewsxcv
Rollup merge of rust-lang#41572 - frewsxcv:bump-mdbook, r=steveklabnik
6fe2c24 
Bump mdbook dep to pick up new 'create missing' toggle feature.

This will avoid obscure Travis CI error messages:

* rust-lang#40290 (comment)

Original mdbook issue:

* https://github.com/azerupi/mdBook/issues/253

mdbook PR:

* https://github.com/azerupi/mdBook/pull/254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@frewsxcv frewsxcv frewsxcv left review comments

Assignees

@aturon aturon

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@3Hren @rust-highfive @bluss @alexcrichton @aturon @frewsxcv @Mark-Simulacrum @arielb1 @bors @carols10cents