search

cessen / ropey

A utf8 text rope for manipulating and editing large texts.
MIT License
1.04k stars 46 forks source link

smaller total api #41

Closed ammkrn closed 3 years ago

ammkrn commented 3 years ago

The error style is sort of verbose, but there were some practical considerations that made something like this seem better than the alternatives. Users will probably only want one error type while retaining the more detailed messages, but I didn't want to do anything stringly typed.

The total versions are in their own impl blocks, and where it would retain the error reporting, the partial versions are just defined as the total().unwrap().

cessen commented 3 years ago

This is awesome! Great work. Do you want me to merge this PR and you can do the rest as a separate PR, or do you want to update this PR? (Or do you want me to do the rest? That's also okay, of course. :-) )

The only thing I want to comment on is that I think the "total function" terminology in the docs may be a bit obtuse. But it's certainly not a big deal, and I'll be doing my own documentation pass anyway, so it doesn't block merging.

(As an aside, I'm also not totally sure "total function" is accurate in this case. For example, does returning an error when you divide by zero really make division a total function, or is it just a way of reporting that you've hit the undefined case? I guess it depends a bit on perspective.)

ammkrn commented 3 years ago

I'm fine with adding more and re-submitting as one PR, I wasn't sure whether the goal was still to limit the extension to just the functions that are used by helix. I'll change the docs to "non-panicking version of...". I guess total is in the eye of the beholder; here in the midwest, x/0 is partial in num -> num, but total in num -> Result<num>.

cessen commented 3 years ago

I wasn't sure whether the goal was still to limit the extension to just the functions that are used by helix.

Ah, got it. I decided to just go in all the way. So if you're up for it, go ahead and do it for everything.

I guess total is in the eye of the beholder; here in the midwest, x/0 is partial in num -> num, but total in num -> Result<num>.

To be fair, I don't have a formal math/CS background, so I may be speaking out of ignorance, ha ha.

Either way, though, I think there are plenty of people that may want to use Ropey but don't have that background either, so sticking to more everyday and/or Rust-centric parlance probably makes more sense for Ropey's documentation.

Thanks so much for doing the leg work on this!

ammkrn commented 3 years ago

Closing in favor of #42.