amber-lang / amber-lang/amber

[Feature] Amber CLI parsing reorganization

Aperta
#1,105 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
compiler enhancement
Lingua principale
Rust
Stelle
5.2k
Fork
145
Merge medio
5g 3h
PR unite (30g)
7

Descrizione

Ref: https://github.com/amber-lang/amber/pull/1086

With the help of ChatGPT I did a recap of what we should about this topic.

Here’s a proposal for reorganizing the whole `std/cli` implementation and test suite.

### 1. Split the module into clearly separated responsibilities

Right now `src/std/cli.ab` mixes:

* token classification
* low-level parsing
* mutating consumers
* validation helpers

#### A. Internal token utilities

Private helpers only:

* `is_long_option`
* `is_short_option`
* `is_terminator`
* `split_long_option`
* `find_option_index`

This keeps parsing primitives isolated and easier to reason about.

#### B. Argument normalization

Public entrypoints:

* `cli_args`

This section should only handle normalization of `raw_args`.

#### C. Mutating consumer APIs

Core parsing functions:

* `take_flag`
* `take_option`
* `take_command`
* `take_positional`
* `take_all_positionals`

These functions all share the same behavioral contract:

* mutate `args` only on success
* leave `args` untouched on failure

That contract should be explicitly documented once and consistently enforced.

#### D. Validation helpers

Final validation APIs:

* `cli_assert_empty`

This makes the parser flow much easier to read from top to bottom.

---

### 2. Standardize parser contracts

Instead of repeating this implicitly:

> All `take_*` functions are transactional:
>
> * successful parse => consumes arguments
> * failed parse => leaves input untouched

---

### 3. Reduce duplicated parsing logic

The review correctly points out duplication around:

* `long_eq`
* inline option parsing
* terminator handling

I think the parser would become cleaner if inline parsing were centralized in a single helper that returns a structured result instead of repeatedly recomputing string state.

Something like:

```amber
parse_option_token(arg: Text) -> OptionToken
```

Where `OptionToken` distinguishes:

* long option
* long option with inline value
* short option
* terminator
* positional

That would simplify both `take_flag` and `take_option`.

---

### 4. Reorganize tests by behavioral guarantees

#### Success consumption

* flags
* options
* commands
* positionals

#### Failure non-mutation

Every failing parse should explicitly verify:

* returned failure
* `args` unchanged

This pattern appears repeatedly in review feedback and should become systematic.

#### Terminator behavior

Dedicated section for:

* `--`
* parsing stop semantics
* positional continuation

#### Repeated consumption

Separate tests for:

* repeated flags
* repeated options
* sequential consumption semantics

#### Shell escaping / forwarding

Keep integration-level forwarding tests isolated from stdlib parser tests.

---

### 5. Make mutation semantics visually obvious

A lot of complexity comes from mutating arrays in-place.

I think readability would improve if every consumer followed the same internal structure:

```amber
1. locate token
2. validate token/value
3. only then mutate args
4. return parsed value
```

Right now some branches interleave validation and mutation, which makes failure guarantees harder to audit mentally.

---

### 7. Suggested long-term direction

Eventually this could evolve into a higher-level parser abstraction instead of only procedural helpers.

For example:

```amber
let parser = CliParser(args)
```

with chainable consumption methods.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.