remove methods

[KaranJain21]

Adds the following methods

• checked_remove: checks if the element at a certain index can be removed from the vector, then removes it, shifting all elements after it to the left. If it is not possible to remove, returns a RemoveError
• remove: calls checked_remove, then unwraps
• checked_swap_remove: checks if the element at a certain index can be removed from the vector, then removes it, replacing it with the last element. If it is not possible to remove, returns a RemoveError
• swap_remove: calls checked_swap_remove, then unwraps

All of the above methods return the removed element (if any).

Also add a RemoveError enum, with two variants:

1. LengthOne: to avoid removing from a NonEmpty of length 1
2. IndexOutOfBounds: for index larger than or equal to the length of the NonEmpty

[FintanH]

I'm thinking about what the correct API for this is. What I'm trying to keep in mind is that the non-emptiness of the collection is what we're always trying to maintain.

With that in mind, I think I'd prefer something like:

/// Removes the element found at `index`.
///
/// If the index exists, then the element is returned, alongside the other elements as a `Vec<T>`.
///
/// If the index is out of bounds then the `NonEmpty<T>` is returned as-is.
pub fn remove(self, index: usize) -> Result<(T, Vec<T>), NonEmpty<T>> {
  /* index shenanigans */
}

Note that we cannot guarantee that we have a non-empty collection as soon as we remove, so the caller can call NonEmpty::from_vec, and have their own error types as needed.

For swap_remove, it would be something similar:

pub fn swap_remove(self, index: usize) -> Result<NonEmpty<T>, NonEmpty<T>> {
  /* index shenanigans */
}

Noting here that we can actually return a NonEmpty because we're maintaining the non-emptiness 😁

How do you feel about this? Would you be up for trying out a version of the PR that has this API?

[KaranJain21]

My intention for the currently implemented remove and swap_remove API was to keep it consistent with Vec's remove and swap_remove. I think it's reasonable to do your proposed API for checked_remove and checked_swap_remove, but not sure when I'll get around it.

I personally don't like having to take the NonEmpty struct by value in these methods, and prefer Vec's API of taking a mutable reference. Any reason to not go with the current approach where we just return an error in the case where there's only one element or if index is out of bounds?