sophiajt / new-nu-parser

23 stars 10 forks source link

Refactor shunting yard so it doesn't need `.expect()` #20

Closed tertsdiepraam closed 8 months ago

tertsdiepraam commented 9 months ago

I was looking at the shunting yard algorithm and figured it could be refactored such that it doesn't need any .expect calls anymore. The idea is that the leftmost expression is separated from the stack, so that the stack consists of pairs of op and rhs. This shouldn't change the behaviour (hopefully).

The downside is that getting the correct lhs for an expression is somewhat more complex:

let lhs = expr_stack.last_mut().map_or(&mut leftmost, |l| &mut l.1);

In other words, we get the last expression on the stack or the leftmost if the stack is empty.

This could be abstracted in a new data structure if you want, but I tried to keep the change small for a first contribution. 😄

I've also added a test case with operators with different precedence to test this.

kubouch commented 8 months ago

To make the test pass, you need to also add the result file generated by the test runner src/snapshots/new_nu_parser__test__node_output@math_precedence.nu.snap

tertsdiepraam commented 8 months ago

Ah right, I was wondering how that worked :) Thanks for the hint!

sophiajt commented 8 months ago

Apologies this took so long to get back to you. Thanks for the fix!