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

Made std::optionand std::result more similar #10119

Merged
merged 4 commits into from
Nov 1, 2013

Conversation

Kimundi
Copy link
Member

@Kimundi Kimundi commented Oct 28, 2013

This takes the last reforms on the Option type and applies them to Result too. For that, I reordered and grouped the functions in both modules, and also did some refactorings:

  • Added as_ref and as_mut adapters to Result.
  • Renamed Result::map_move to Result::map (same for _err variant), deleted other map functions.
  • Made the .expect() methods be generic over anything you can
    fail with.
  • Updated some doc comments to the line doc comment style
  • Cleaned up and extended standard trait implementations on Option and Result
  • Removed legacy implementations in the option and result module

@alexcrichton
Copy link
Member

I think that we shouldn't take renaming of option methods very lightly any more. Has the transition from map_default to map_or been discussed and agreed upon?

Also in the future, I know it's tough and I always forget to do this as well, but it's always nice for reviewing if reorganization of code is separated (by commits) from refactoring/renaming and additions to code. It's tough to see here that the only method on option that changed is map_default. I'd be interested in the amount of fallout that the change had, but right now it's all mixed in with the Result changes as well.

Other than that though, I like how this pretty much makes the apis of option and result the same. I'm in favor of all the changes to Result, I think that map_default may need more discussion though.

I'd r+ a pull request which didn't modify map_default though!

@thestinger
Copy link
Contributor

We previously came to the consensus that using default for methods unrelated to Default should be changed.

@Kimundi
Copy link
Member Author

Kimundi commented Oct 31, 2013

I updated the PR to not include the map_default -> map_or change for now. I'll make a separate PR for that once this lands.

@alexcrichton
Copy link
Member

I talked with @Kimundi on IRC, and I like where this is going, but I'm personally a little uneasy about all of the err_* variants of functions. He's going to go play around with the idea of a fn ok(self) -> Option<T> function and see if we can reduce the surface area of the api here.

Cleaned up the source in a few places

Renamed `map_move` to `map`, removed other `map` methods

Added `as_ref` and `as_mut` adapters to `Result`

Added `fmt::Default` impl
Made both types implement more standard traits in a nicer way

Derived more traits
@Kimundi
Copy link
Member Author

Kimundi commented Nov 1, 2013

@alexcrichton: A buildbot failed with a runtime abort, so I used the additional time to update this PR with as much cleanup as possible without touching the _err functions

bors added a commit that referenced this pull request Nov 1, 2013
This takes the last reforms on the `Option` type and applies them to `Result` too. For that, I reordered and grouped the functions in both modules, and also did some refactorings:

- Added `as_ref` and `as_mut` adapters to `Result`.
- Renamed `Result::map_move` to `Result::map` (same for `_err` variant), deleted other map functions. 
- Made the `.expect()` methods be generic over anything you can
  fail with.
- Updated some doc comments to the line doc comment style
- Cleaned up and extended standard trait implementations on `Option` and `Result`
- Removed legacy implementations in the `option` and `result` module
@bors bors closed this Nov 1, 2013
@bors bors merged commit 415a043 into rust-lang:master Nov 1, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Dec 29, 2022
…Frednet

fix codeblocks in the document

While I've looked into rust-lang/rust-clippy#9990, I found broken code blocks in the document.

This patch not only improves visibility, but also fixes a potential bug. When a lint description ends with code block, the string will have three backquotes at the end.
Since the current implementation prints the default value immediately after that, the markdown renderer is unable to properly close the code block.

e.g. `arithmetic_side_effects`, we can see code block is not rendered properly, and (I think) it's bit hard to understand what ``"defaults to `[]`"`` is meant.

![2022-12-26_01-51](https://user-images.githubusercontent.com/14945055/209476342-4d0b2e18-44b6-4c74-8c3c-4f4f0904e8ca.png)

In this PR, it will be rendered as:

![image](https://user-images.githubusercontent.com/14945055/209476353-07587b86-1100-445f-946d-41f62f741e51.png)

changelog: none

r? `@xFrednet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants