fold range implementation

[Stargateur]

Closes #1393.

There is main difference with #1402

• We don't agree with range interpretation, This PR try to follow Consolidate parser variants using ranges (e.g. many0, many_m_n) #1393 (comment)
• I simply implement many with fold
• I do not preallocate for many, I believe most of the time it's probably worst, I can't really be sure of that, but allocate a giant vector when the first item fail doesn't seem good. I think we should not p reallocate if the following could fail.
• Is straightforward

I think we should have some clarification of range interpretation before continue any further.

• implement fold
• implement many with Extend trait Closes Support custom containers for many functions #1409
• implement NomRangeBounds
• implement try_fold Closes try_fold support #1411
• put fold and try_fold in their own module
• Try to share code between fold and try_fold ? use try_fold to implement fold (check if optimized away) ? use a macro ?
• rework test (I mostly copy past what cenodis did in [WIP] Feature multi range #1402)
• check doc (same I mostly copy past cenodis ^^ I will try to add cenodis as author of this PR in commit but my git jutsu is not that advanced)
• Add guide to explain more clearly the range concept of this feature

[cenodis]

for example ..5 is 0..5 for me but I think for [WIP] Feature multi range #1402 it's 5..5 (not sure)

untrue. ..5 is interpreted as 0..5.

I simply implement many with fold

That breaks nom convention because many now returns the Fold error variant. Each basic nom parser should return its own variant.

I do not preallocate for many

Preallocation in #1402 was done because the old implementations of many* also preallocate. But that is so easy to change its not a dealbreaker for either PR.

we don't agree with interpretation of 0..5 or 0..=5 etc...

When I use many like this (error handling code omitted):

let res = many(0..=5,
  //snip
);

The user would expect the resulting Vec to have between 0 and 5 elements (i.e. (0..=5).contains(res.len()) should be true). Which is also how the old many_m_n parser works.
What you proposed in #1402 would mean that many could return a Vec with 6 elements. Which is misleading and goes against the common interpretation of a range (The length of the Vec is not contained within the range).

Edit:
This also does not cover the case when the range is a single usize value. So this is impossible with your implementation:

many(5
  //snip
);

[Stargateur]

The user would expect the resulting Vec to have between 0 and 5 elements (i.e. (0..=5).contains(res.len()) should be true). Which is also how the old many_m_n parser works.
What you proposed in #1402 would mean that many could return a Vec with 6 elements. Which is misleading and goes against the common interpretation of a range (The length of the Vec is not contained within the range).

allow me to strongly disagree. That completely the opposite and I'm sure of it. See my comment on #1393 assert_eq!((0..=5).count(), 6). Your contains code doesn't work. Or we strongly disagree about range interpretation. Again follow up on the issue.

How do you handle 0..=usize::MAX ? my code do what is expected.

This also does not cover the case when the range is a single usize value. So this is impossible with your implementation:

I have literally no idea how to interpret 5 correctly, the purpose is to handle range, 5 have absolutely no consensus of what it should be. I see it as an unclear feature.

Preallocation in #1402 was done because the old implementations of many* also preallocate. But that is so easy to change its not a dealbreaker for either PR.

yes I guess, doesn't concern my reason.

That breaks nom convention because many now returns the Fold error variant. Each basic nom parser should return its own variant.

Since when there is such convention ? I'm sure I can find few parser that don't do that.

[Stargateur]

https://github.com/Geal/nom/pull/1407/checks?check_run_id=3712855476

std::usize::MAX is depreciated since 1.43

[epage]

Preallocation in #1402 was done because the old implementations of many* also preallocate. But that is so easy to change its not a dealbreaker for either PR.

Supporting any collection that impls Extend will force us to drop pre-allocation anyways: #1409

[epage]

I have literally no idea how to interpret 5 correctly, the purpose is to handle range, 5 have absolutely no consensus of what it should be. I see it as an unclear feature.

Part of my hope with #1393 was that N => N..=N (I did have some bugs in the example, just fixed).

This allows us to fold count into many and consistently provide exact-count variants of all parsers.

Personally, I think the intent is clear by having a single number be a placeholder for an inclusive range.

[Stargateur]

Part of my hope with #1393 was that N => N..=N (I did have some bugs in the example, just fixed).

This allows us to fold count into many and consistently provide exact-count variants of all parsers.

Personally, I think the intent is clear by having a single number be a placeholder for an inclusive range.

I'm still not convince, I did it but I found it's very limited, can you extend what you mean with count() ?

[Stargateur]

this is not good 2..=1 should be valid