Skip to content

Rewrite `pipelines::collect` to be a recursive descent parser

Michael Aaron Murphy requested to merge huntergoldstein:pipeline_rewrite into master

Created by: huntergoldstein

Problem: The previous version of pipelines::collect was a dragon's nest of bitflags and break <label>. It was difficult to extend for features such as #132 (closed) and #133 (closed).

Solution: pipelines::collect has been replaced by pipelines::Collector, which encapsulates the logic of pipelines::collect in a recursive descent parser. Additionally, instead of taking a mutable reference to an Option<&str>, pipelines::Collector::run returns a Result<...>.

Changes introduced by this pull request:

  • Removed pipelines::collect
  • Added pipelines::Collector and replaced instances of pipelines::collect with the logical equivalent for pipelines::Collector.

Drawbacks:

  • The error messages are still of questionable quality. The main problem is being constrained by peg: the error messages have to be &strs according to the documentation, which is just mind boggling.
  • There's still a good amount of complexity, cyclomatic complexity warning notwithstanding

TODOs:

  • Add tests for obscure redirects like &>> and ^>>

State: WIP just so that it has more time for review. Also see TODOs.

Other: In addition to the mentioned issues I'm trying to figure out what to tackle next. Replacing PEG is a must, but there's also statements.rs which needs a similar makeover.

Merge request reports