Checked resolve

[eirnym]

Implementation notes:

• Error kind is important for distinguish outside library, which exact error has been occurred. Checking a formatted message is not an option as it's non-stable as it possibly could be.
• Error message is quite dummy at the moment of implementation as it's the least important for me.

[yescallop]

Before we continue I would like to see some prior art on this topic, considering the complexity this patch adds. Is there any URI/URL/path handling code known to signal an underflow in path resolution?

[eirnym]

I've tried to add as minimal complexity as possible to make feature done. 3 unit expected tests moved from pass to fail, other tests are just a copy.

[eirnym]

I work on a tool for JSON Schema handling, and if a JSON Schema reference underflows, I notify a user which exact node causes an issue. I don't use messages from libraries is they provide enough context information I need or error message is already included in mine.

[eirnym]

@yescallop What do you think on that?

[yescallop]

I think it would be fine to provide an API for detecting path underflows, but probably not in this particular form. I prefer not to add more methods to Uri as this may add to users' cognitive load. One option may be adding Resolver and Normalizer structs with which users can customize the strategy for resolution and normalization. Instead of returning an error on underflow, we might as well return a boolean indicating whether an underflow happened, like what i32::overflowing_add does.

That said, I don't think such an API would land anytime soon. My current priority is to prepare for a full-featured 0.2 release. I'll try to keep the crate simple for the moment.

[eirnym]

My application is dependant on this functionality and I add unchecked methods only when I add quirks, not as a normal way of work. You proposed to implement resolve_checked, and I've did it with minimal changes possible

This I'd like to agree and disagree with your statements.

Points where I agree with you:

1. Direct functionality of Uri struct should be split into separate traits like you've already proposed, but I'd like that to add it after PR would be merged as I see it as a refactoring, not a blocker.

Points I don't agree with:

1. (un)checked methods are quite common in Rust and they're somewhat mixed up.
2. Custom traits implementation is impossible in Rust if a trait or struct doesn't belong to a crate where implementation is defined.
3. If custom implementation was possible, resolver and checked resolver should use the exact same algorithm with a check only in a single place.
4. There's no point to process further if underflow is detected, and idiomatic way in Rust is to report that using Result.

[eirnym]

PS: if you worry about optimization, matching would be the same optimal as checking boolean

[yescallop]

Hi, @eirnym. Thanks for your input on this feature! I recently created #15 as an alternative implementation and was wondering if you would like to review it.

The main changes I made is that I added a method ResolveError::is_path_underflow instead of exposing the ResolveErrorKind enum, as I think the latter option exposes too much info. Would you think that this is okay with your use case?

[eirnym]

@yescallop Thank you for returning to me with this.

Firstly, I'd like to keep this PR and I don't like cloning them. You have permission to edit it, at least GitHub asked me give you such permission. I updated my code to match master and updated documentation.

Secondly, it's better for developer to decide what to do with each error kind, without guessing what happened in a some third party dependency. For example, you won't provide translations, context and cetera for each error and it's kind. In languages with exceptions there's a rule: if you don't know what exactly application needs to do with it, yield/throw it further. Same with errors in languages like C and Rust.

[eirnym]

The same if you don't like specific change, it's better to start discussion instead of making a copy of a PR.

[yescallop]

I understand that this feature may be critical to your application, but I would prefer not to include it in this crate. You might consider maintaining a fork of the crate. Thanks for your understanding.

[eirnym]

It's sad to read this without reasoning form your side except (possibly) emotions

[eirnym]

I request not to copy my work and use it as yours

[yescallop]

I do open source because it's fun, and when it's not fun I have the right to toss it aside. It might be because English is not our first language, but I sense a huge communication barrier between us. I did not copy your work and use it as mine, because I had thought of the exact same solution as yours before you did the PR since the changes are all trivial.

[yescallop]

Also, I would appreciate it if you could stop pushing me too much. You might not have intended it, but your tone has made me think so. I'll return to your issues next time, when I get less busy with work.

[eirnym]

I'm sorry if you took it this way

[eirnym]

This was my reaction to the way you communicated here.

Below are my expectations for every PR I made in every project.

I expect open communication why what and where with explanations and open discussion.

I expect asking me to merge/rebase main branch, not to create a new one with same changes by clearing my name from these changes.

I expect people propose a patch in a comments if they would like to be changed in my code.