-
Notifications
You must be signed in to change notification settings - Fork 12
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
Improve error handling ergonomics (and docs around it) #62
Conversation
In a new project, after `cargo add ada-url`, now compiles successfully
`set_port` returns a `Result` we `must_use`. Fixes the compiler warning.
This implements `std::error::Error` for a newly introduced error type. We cannot implement that trait on `()` in this crate, so cannot offer consumers convenient error handling via `?` etc. (`anyhow`, ...) without a dedicated error type we own.
Found by temporarily adding ```rust ``` to `lib.rs`. Removed it again because not all lints are solved (I only applied the more useful ones, which should benefit library consumers at no costs), and I didn't think putting `#[allow(...)]` throughout was a good idea.
Files in `examples/` will be compiled by `cargo test`. Hence, uncompilable code will be caught from now on. One can `cargo run --example simple` to see the `println!` output. This allows users to get started hacking quickly.
I noticed the example in the README not compiling, for two and a half reasons (2 errors, 1 warning). I fixed those, but then noticed the To keep example code from breaking again, I moved it to I also applied a bunch of fixes to lints (Rust and Markdown) throughout, making this PR slightly messy. The commits should be mostly well-behaved though, so we can drop commits/split the PR if you'd like. |
Can you revert the |
Now passes `just all`
Yeah I saw CI failing and then noticed Line 54 in be5c260
I adjusted the example to demonstrate ergonomics when
Does that work for you or would you still like that part reverted? |
This seems like a breaking change, and I wonder if it's worth it. Can you revert it for the time being? |
Ok, reverted it (passes It's pretty neat to have This PR is now a bit messy (and with less content). I didn't rebase and/or squash in order to keep history so that the implementation can be retrieved later on. |
Thanks for the hard work. I'll merge and release a new version. |
Unfortunately clippy is failing now |
I'm pretty sure that's unrelated to this PR. It fails at The last Clippy run on Running the equivalent command locally on Rust 1.76 or higher (set $ git clean -xfd && sed -i 's/^channel = .*/channel = "1.76"/' rust-toolchain.toml && cat rust-toolchain.toml && git switch --detach be5c260 && cargo hack clippy --feature-powerset -- -D warnings
Removing target/
[toolchain]
channel = "1.76"
profile = "default"
M rust-toolchain.toml
HEAD is now at be5c260 build: don't force a specific compiler/version (#60)
info: running `cargo clippy --no-default-features -- -D warnings` on ada-url (1/10)
Compiling libc v0.2.147
Compiling proc-macro2 v1.0.66
Compiling memchr v2.6.2
Compiling unicode-ident v1.0.11
Compiling regex-syntax v0.8.2
Compiling syn v1.0.109
Compiling link_args v0.6.0
Compiling convert_case v0.4.0
Compiling aho-corasick v1.0.5
Compiling quote v1.0.33
Compiling jobserver v0.1.26
Compiling cc v1.0.83
Compiling regex-automata v0.4.3
Compiling derive_more v0.99.17
Compiling regex v1.10.2
Compiling ada-url v2.2.1 (/home/alex/dev/ada-url-rust)
error: the semantics of `Borrow<T>` around `Hash` can't be satisfied when both `Borrow<str>` and `Borrow<[u8]>` are implemented
--> src/lib.rs:743:6
|
743 | impl hash::Hash for Url {
| ^^^^^^^^^^
|
= note: the `Borrow` semantics require that `Hash` must behave the same for all implementations of Borrow<T>
= note: however, the hash implementations of a string (`str`) and the bytes of a string `[u8]` do not behave the same ...
= note: ... as (`hash("abc") != hash("abc".as_bytes())`
= help: consider either removing one of the `Borrow` implementations (`Borrow<str>` or `Borrow<[u8]>`) ...
= help: ... or not implementing `Hash` for this type
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#impl_hash_borrow_with_str_and_bytes
= note: `#[deny(clippy::impl_hash_borrow_with_str_and_bytes)]` on by default
error: could not compile `ada-url` (lib) due to 1 previous error
error: process didn't exit successfully: `/home/alex/.rustup/toolchains/1.76-x86_64-unknown-linux-gnu/bin/cargo clippy --manifest-path Cargo.toml --no-default-features -- -D warnings` (exit status: 101) The same command on 1.75 succeeds. See also rust-lang/rust-clippy#11781 . We could address this lint, ignore it, fix Rust version to 1.75, ... ? |
Result
Error
forSetterResult
clippy
lintsexamples/