search

zajrik / option_result

A lightweight Dart library for Rust-like Option/Result types and associated pattern matching
https://pub.dev/packages/option_result
MIT License
10 stars 1 forks source link

Single public library #3

Closed manuel-plavsic closed 7 months ago

manuel-plavsic commented 10 months ago

I personally think that it does not make much sense to include multiple public libraries for this package since it is a relatively small one, especially since one is likely to use Option and Result together. In general, I also think it is more intuitive if there is just one library to import a component of some package from.

zajrik commented 7 months ago

Hi Manuel,

I apologize, I never saw any notifications for all of your PRs being opened or committed to until now when you've closed them all. I don't know how I managed to miss them as I am generally pretty on top of my GitHub notifications. Thank you for taking the time to work on this.

I've taken a cursory glance at the PRs and I'm seeing a lot of changes that don't really fit well for me, but some that do. When I get the chance I will see about adopting the changes I feel good about if I can find the opportunity. For some of them it's harder to see what changes you've made due to dartfmt being used and ruining the diffs (I can't stand dartfmt's style, I can't even really read code formatted with it, my eyes can't follow it at all lol), so that may present a challenge.

manuel-plavsic commented 7 months ago

Hi Zack,

No problem ;) sometimes we are so busy and it can happen. You are welcome :)

Due to absence of feedback, I eventually ended up forking your library. Here you can find my fork https://pub.dev/packages/nidula. I added some nice sections like VSCode snippets (in my experience, they can replace the need for the value getters val, value, ...). Also, I reimplemented the parallel to the Rust's try operator: it is the try_ method instead of unwrap. This way, like in Rust, unwrap simply panics if called on an Err or None value. I also made it compile time safe by introduction of tokens (although slightly "ugly", it definitely serves its purpose: it is much better to avoid propagated error type mismatches at runtime). Another thing i changed is I added some commas to the source code, so when the code is formatted it is easier to read the pattern-matching parts of code (every case has its own line instead of condensing them together). You can have a look at my implementation.

Initially I thought it would be better to exclude nullable types (see this PR https://github.com/zajrik/option_result/pull/6), however I later changed my mind. It is fine to keep nullable types. Through extensions, I added methods e.g. toNullable just to Option<T extends Object>.

Regarding dartfmt, would it be possible to disable it in .vscode/settings.json and maybe also include your indentation preferences? You should definitely include it in the repo, otherwise it is hard for others to contribute to your repo.