Move the command language interpretation system into its own module #159

Vectornaut · 2026-06-15T19:31:57Z

Vectornaut commented

2026-06-15 19:31:57 +00:00

The branch to be merged addresses issue #154 by moving the the system for interpreting commands as actions into its own module. It positions the action system as more fundamental than the language interpretation system: the language module uses lots of stuff from the action module, while the action module barely uses anything from the language module.

Snags in factoring

The separation isn't as clean as I'd like it to be, but we could see that as a point in favor of merging: on the branch to be merged, poorly-factored code stands out more because it's in the wrong module. In one case, this motivated me to find a better organization. Here are the main snags I hit.

The `DisplayValue` action

The DisplayValue action is so deeply intertwined with the internals of the interpretation system that I decided to move it to the new language module rather than keeping it in the action module with the rest of the default actions. The alternative would've been making lots of interpretation system structures public just for the sake of this one action.

As we've discussed, we may eventually switch to a design where the DisplayValue action would disappear entirely, because top-level actions would be allowed to have non-unit return values. The behavior currently encoded in DisplayValue would instead become an aspect of the command console view.

Recording constructors (now better organized)

On the main branch, the CreateElement and CreateRegulator actions have alternative constructors that both create an object and record it in a given Environment so that it can be resolved by other descriptors in the same command. On the branch to be merged, Environment is part of the language module, so it's awkward to put these constructors in the action module. I therefore turned them into methods of Environment (commit 502746f), which in hindsight seems like a better organization anyway.

The `RegulatorOperator` enumeration

The RegulatorOperator enumeration is the only thing from the language module which is used in the action module. I've decided that it belongs in the language module, because it's an abstract syntax tree component based directly on concrete syntax rules. However, the DoRegulatorOperation action uses it to determine which action to do. This doesn't feel like a big deal to me, so I left it as-is rather than trying to reorganize.

The branch to be merged addresses issue #154 by moving the the system for interpreting commands as actions into its own module. It positions the action system as more fundamental than the language interpretation system: the language module uses lots of stuff from the action module, while the action module barely uses anything from the language module. ### Snags in factoring The separation isn't as clean as I'd like it to be, but we could see that as a point in favor of merging: on the branch to be merged, poorly-factored code stands out more because it's in the wrong module. In one case, this motivated me to find a better organization. Here are the main snags I hit. #### The `DisplayValue` action The `DisplayValue` action is so deeply intertwined with the internals of the interpretation system that I decided to move it to the new language module rather than keeping it in the action module with the rest of the default actions. The alternative would've been making lots of interpretation system structures public just for the sake of this one action. As we've discussed, we may eventually switch to a design where the `DisplayValue` action would disappear entirely, because top-level actions would be allowed to have non-unit return values. The behavior currently encoded in `DisplayValue` would instead become an aspect of the command console view. #### Recording constructors (now better organized) On the main branch, the `CreateElement` and `CreateRegulator` actions have alternative constructors that both create an object and record it in a given `Environment` so that it can be resolved by other descriptors in the same command. On the branch to be merged, `Environment` is part of the language module, so it's awkward to put these constructors in the action module. I therefore turned them into methods of `Environment` (commit 502746f), which in hindsight seems like a better organization anyway. #### The `RegulatorOperator` enumeration The `RegulatorOperator` enumeration is the only thing from the language module which is used in the action module. I've decided that it belongs in the language module, because it's an abstract syntax tree component based directly on concrete syntax rules. However, the `DoRegulatorOperation` action uses it to determine which action to do. This doesn't feel like a big deal to me, so I left it as-is rather than trying to reorganize.

Vectornaut added 2 commits

2026-06-15 19:31:58 +00:00

Move language interpretation into its own module 63e976816d

Move recording constructors into Environment

/ test (pull_request) Successful in 3m51s

Details

4b18e9f432

Vectornaut added 1 commit

2026-06-18 18:13:57 +00:00

Subdivide the language module

/ test (pull_request) Successful in 3m52s

Details

caa799519b

Vectornaut commented

2026-06-18 18:19:51 +00:00

I've subdivided the language module into syntax and semantics submodules (commit 6ab4308), sort of like we planned during Monday's meeting. I decided to keep language as a container for syntax and semantics for two reasons.

It communicates that the syntax and semantics modules are closely related.
It creates a natural home for the top-level run_script method and the end-to-end tests, which span all stages of parsing and interpretation.

I've subdivided the language module into syntax and semantics submodules (commit 6ab4308), sort of like we planned during Monday's meeting. I decided to keep `language` as a container for `syntax` and `semantics` for two reasons. - It communicates that the syntax and semantics modules are closely related. - It creates a natural home for the top-level `run_script` method and the end-to-end tests, which span all stages of parsing and interpretation.

glen reviewed

2026-06-18 21:05:28 +00:00

app-proto/src/action.rs Outdated

					
				@ -8,3 +5,1 @@

				    fmt,

				    fmt::{Debug, Display, Formatter},

				    iter,

				    fmt::{self, Display, Formatter},

What does self mean in the context of this use statement?

What does `self` mean in the context of this `use` statement?

By example: the statement

use fmt::{self, Display, Formatter};

is equivalent to the statements

use fmt;
use fmt::Display;
use fmt::Formatter;

By example: the statement ```rust use fmt::{self, Display, Formatter}; ``` is equivalent to the statements ```rust use fmt; use fmt::Display; use fmt::Formatter; ```

glen marked this conversation as resolved

glen reviewed

2026-06-22 21:51:23 +00:00

app-proto/src/language/syntax.rs Outdated

					
				@ -0,0 +122,4 @@

				#[grammar = "language/syntax.pest"]

				pub struct LanguageParser;

				impl<'a> From<Pair<'a, Rule>> for Statement {

There are all of these Pair<...> types floating around which I think are the types of the things called "captures" when you actually try to run the pest-generated parser. The typename is not very suggestive in terms of what we are doing with these things. Is there any chance that the equivalent of a typedef could be made so that we could write (say) Capture for Pair<Rule> and then I guess we would need some similar name like Captured<'a> for Pair<'a, Rule> or something like that. Would that be workable? Other than that (and the unfortunate current need for both Environments and Assembly entities, which I think we are resigned to at the moment,) I don't see any code issues. Running the system.

There are all of these `Pair<...>` types floating around which I think are the types of the things called "captures" when you actually try to run the pest-generated parser. The typename is not very suggestive in terms of what we are doing with these things. Is there any chance that the equivalent of a typedef could be made so that we could write (say) `Capture` for `Pair<Rule>` and then I guess we would need some similar name like `Captured<'a>` for `Pair<'a, Rule>` or something like that. Would that be workable? Other than that (and the unfortunate current need for both Environments and Assembly entities, which I think we are resigned to at the moment,) I don't see any code issues. Running the system.

Done in commit 01e62be!

Done in commit 01e62be!

glen marked this conversation as resolved

Vectornaut changed title from ~~Move the command language interpretation system into its own module~~ to WIP: Move the command language interpretation system into its own module

2026-06-23 18:45:34 +00:00

Vectornaut commented

2026-06-23 18:50:43 +00:00

Like we discussed during Monday's meeting, I've put this pull request on hold until we address issues #157 and #160, because those issues could obscure any regressions that this pull request might introduce in the test assembly loading scripts. Once pull request #161 is merged, I'll rebase this branch onto it and then put this pull request back in review mode.

Vectornaut force-pushed language-module from caa799519b

/ test (pull_request) Successful in 3m52s

Details

to 6ab43081cb

/ test (pull_request) Successful in 3m48s

Details

2026-06-23 22:49:46 +00:00

Compare

Vectornaut added 1 commit

2026-06-23 23:08:40 +00:00

Alias the type Pair<'a, Rule>

/ test (pull_request) Successful in 3m45s

Details

01e62be2ea

This turns the confusing name `Pair` into something more descriptive,
and it gets rid of the repetitive type parameter too.

Vectornaut changed title from ~~WIP: Move the command language interpretation system into its own module~~ to Move the command language interpretation system into its own module

2026-06-23 23:09:26 +00:00

Vectornaut commented

2026-06-23 23:11:04 +00:00

I've rebased onto pull request #161 and made the requested type alias, so this branch should be ready for final review!

glen reviewed

2026-06-25 04:52:21 +00:00

app-proto/src/language.rs

					
				@ -0,0 +13,4 @@

				    )?;

				    for statement_capture in captures {

				        Statement::from(statement_capture)

				            .interpret(&mut Environment::new(state), None)?

Why should the outside caller of interpret care about this Environment object? I.e., why doesn't it just take the state and generate the Environment internally? Is there a use case in which either it would be important to pass in something other than a new Environment, or where the caller would examine the mutated Environment after the call?

Why should the outside caller of `interpret` care about this Environment object? I.e., why doesn't it just take the state and generate the Environment internally? Is there a use case in which either it would be important to pass in something other than a new Environment, or where the caller would examine the mutated Environment after the call?

The code you're looking at interprets a statement that sits at the root of an abstract syntax tree. Statements can also appear deeper in the tree, as the details of a detailed descriptor. In this case, the statement has to be interpreted within a pre-existing environment rather than a fresh one.

The code you're looking at interprets a statement that sits at the root of an abstract syntax tree. Statements can also appear deeper in the tree, as the [details](https://code.studioinfinity.org/StudioInfinity/dyna3/src/commit/01e62be2eade54841e90ee7dadac7194e0529f45/app-proto/src/language/syntax.rs#L51) of a detailed descriptor. In this case, the statement has to be [interpreted](https://code.studioinfinity.org/StudioInfinity/dyna3/src/commit/01e62be2eade54841e90ee7dadac7194e0529f45/app-proto/src/language/semantics.rs#L525) within a pre-existing environment rather than a fresh one.