Actions were inspired by Python and Python does not implicitly default
any field when an action is given. From a Builder API perspective, this
seemed fine because we tend to focus the Builder API on giving the user
all information so they can make their own decisions. When working on
the Derive API, this became a problem because users were going to have
to migrate from an implied default to an explicit default when a common
default is good enough most of the time. This shouldn't interfere with
Builder users getting more details when needed.
This also highlighted two problems
- We set the index for defaults
- We don't debug_assert when applying conditional requirements with a
default present
This round out the new style actions and allow us to start deprecating
occurrences.
As part of an effort to unify code paths, this does change flag parsing
to do splits. This will only be a problem if the user enables splits
but we'll at least not crash. Once we also address #3776, we'll be able
to have envs all work the same.
This is the minimum set of actions for the derive to move off of
`parse`. These are inspired by Python's native actions.
These new actions have a "unified" behavior with defaults/envs. This
mostly means that occurrences aren't tracked. Occurrences were used as
a substitute for `ValueSource` or for counting values. Both cases
shouldn't be needed anymore but we can re-evaluate this later if needed.
If we felt this was important long-term, we should fix this outside of
the Action. Since we might be changing up occurrences (#3772), we can
probably get away with a hack.
This changes how occurrences and values are grouped for multiple values.
Today, it appears as a bug. If we move forward with #3772, then this
can make sense.
When creating `PendingValues`, I can't have the lifetime. I could make
it a `Cow` but decided to hold off instead since we don't need this
right now. Maybe by the time we do need it, we'll have another way of
doing this.
There is a default_missing_vals case which is slightly different because
its not actually a default but closing out the remaining argument that
was started in last iteration.
Before, if we were in trailing values that aren't delimite, we wouldn't
respect this flag and end processing of the value, now we do.
This also has a slight perf benefit of us only splitting the value if
the delimiter is present. We checked for the delimiter anyways, so
doing it first removes a slight bit of work.
I also feel this helps clarify the intended behavior and ooches us
towards a unified code path for actions.
I wrote these tests expecting to highlight a bug but it turns out things
were structured just right to not exhibit it. The fact that the code
looks like its broken is a problem, so I restructured it (put it first,
changed the source) so it doesn't look suspicious anymore.
We were independently starting occurrences and starting value groups.
Now we do them at the same time.
COMPATIBILITY: This changes us from counting occurrences per positional
when using `multiple_values` to one occurrence. This is user visible
and tests were written against it but it goes against the documentation
and doesn't quite make sense.
Inferred flags can make it hard for a future action to trigger behavior
off of the selected alias, like we might want to do for negations, so we
are now translating to the intended arg.
This will also help for debugging.
