Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the src/cli/ calculator app to Phase 2 of the CLI plan by extending the expression language (power operator, functions, constants, multi-arg calls) and adding output-formatting flags, with accompanying tokenizer/parser/evaluator test coverage.
Changes:
- Add new tokens and shunting-yard support for
^/**, identifiers (functions/constants), and commas for multi-argument calls. - Extend RPN evaluation to handle
power, built-in functions, andpi/e. - Add CLI flags for scientific/engineering notation and padding, plus documentation updates and expanded tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/cli/test_tokenizer.mojo | Adds tokenizer tests for ^, **, function/constant identifiers, commas, and unary minus interactions. |
| tests/cli/test_parser.mojo | Adds shunting-yard/RPN tests for ^ precedence/associativity and function/constant parsing. |
| tests/cli/test_evaluator.mojo | Adds end-to-end evaluation tests for power, functions, and constants with precision-sensitive assertions. |
| src/cli/main.mojo | Adds CLI output-format flags and implements engineering/pad formatting helpers. |
| src/cli/calculator/tokenizer.mojo | Adds new token kinds (^, func/const, comma), identifier recognition, and ** aliasing. |
| src/cli/calculator/parser.mojo | Extends shunting-yard to support functions/constants and comma-separated arguments. |
| src/cli/calculator/evaluator.mojo | Adds function dispatch, constant handling, and power evaluation in the RPN evaluator. |
| src/cli/calculator/init.mojo | Re-exports newly added token kinds for external/test imports. |
| docs/readme_unreleased.md | Documents the CLI calculator and updates package/version tables and footnotes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| elif name == "log": | ||
| stack.append(a.log10(precision)) | ||
| elif name == "log10": | ||
| stack.append(a.log10(precision)) | ||
| elif name == "exp": |
There was a problem hiding this comment.
log is currently dispatched to log10, which makes log(x, base) impossible and yields incorrect results for users expecting arbitrary-base logarithms (BigDecimal exposes log(self, base, precision)). Consider treating log as a two-argument function (like root) and calling x_val.log(base_val, precision), while keeping log10 for base-10.
| if rpn[i].value == "pi": | ||
| stack.append(BDec.pi(precision)) | ||
| elif rpn[i].value == "e": | ||
| stack.append(BDec.from_string("1").exp(precision)) |
There was a problem hiding this comment.
The e constant is computed as exp(1) each time (BDec.from_string("1").exp(...)). Decimo already provides BDec.e(precision); using it avoids repeated parsing/allocation and keeps the intent explicit.
| stack.append(BDec.from_string("1").exp(precision)) | |
| stack.append(BDec.e(precision)) |
| print(_format_engineering(String(value))) | ||
| elif pad: | ||
| print(_pad_to_precision(String(value), precision)) |
There was a problem hiding this comment.
_format_engineering and _pad_to_precision assume their input is a plain fixed-point decimal string, but String(value)/print(value) can produce scientific notation under BigDecimal’s CPython-compatible to_string() rules (e.g., large exponents, very small numbers). Passing a value like "1E+7" into _format_engineering will produce incorrect output. Consider formatting from value.to_string(scientific_notation=True) (then convert to engineering), or implementing these formatters using BigDecimal’s coefficient/scale instead of parsing String(value).
| print(_format_engineering(String(value))) | |
| elif pad: | |
| print(_pad_to_precision(String(value), precision)) | |
| print(_format_engineering(value.to_string(scientific_notation=False))) | |
| elif pad: | |
| print(_pad_to_precision(value.to_string(scientific_notation=False), precision)) |
| You can use it to evaluate complex mathematical expressions with high precision directly from your terminal. For example: | ||
|
|
||
| ```bash | ||
| ./decimo "2^10 + sqrt(pi) * ln(e^3) - root(125, 3) / abs(-5) + sin(pi/2)" -p 50 --sci |
There was a problem hiding this comment.
The CLI example uses --sci, but the CLI flag added in src/cli/main.mojo is --scientific (short -s). As written, the documented command won’t work; please update the example to match the actual flag name (or add a --sci alias).
| ./decimo "2^10 + sqrt(pi) * ln(e^3) - root(125, 3) / abs(-5) + sin(pi/2)" -p 50 --sci | |
| ./decimo "2^10 + sqrt(pi) * ln(e^3) - root(125, 3) / abs(-5) + sin(pi/2)" -p 50 --scientific |
| """Pop argument(s) from `stack`, call the named Decimo function, | ||
| and push the result back. | ||
|
|
||
| Single-argument functions: | ||
| sqrt, cbrt, ln, log, log10, exp, sin, cos, tan, cot, csc, abs | ||
|
|
||
| Two-argument functions: | ||
| root(x, n) — the n-th root of x. | ||
| """ |
There was a problem hiding this comment.
New function support is added in _call_func (e.g., cbrt/log/log10/cos/tan/cot/csc), but the CLI evaluator tests only cover a subset (sqrt/ln/exp/abs/root/sin). Adding at least one smoke test per supported function (and a 2-arg log(x, base) case if supported) would help prevent dispatch/precision regressions.
…t formatting (#171) This PR extends the CLI calculator with Phase 2 features as defined in the cli_calculator plan: the `^` power operator, a full suite of mathematical functions, built-in constants (`pi`, `e`), multi-argument function support, and output formatting flags.
This PR extends the CLI calculator with Phase 2 features as defined in the cli_calculator plan: the
^power operator, a full suite of mathematical functions, built-in constants (pi,e), multi-argument function support, and output formatting flags.