Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested internally tagged enums wrong line report #802

Closed
soywod opened this issue Oct 25, 2024 · 3 comments
Closed

Nested internally tagged enums wrong line report #802

soywod opened this issue Oct 25, 2024 · 3 comments

Comments

@soywod
Copy link

soywod commented Oct 25, 2024

Given the following nested structure (small extract from a user config):

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
struct Config {
    backend: BackendConfig,
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
enum BackendConfig {
    Imap(ImapConfig),
    Smtp(SmtpConfig),
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
struct ImapConfig {
    auth: ImapAuthConfig,
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
enum ImapAuthConfig {
    Password(PasswordConfig),
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
struct SmtpConfig {
    auth: SmtpAuthConfig,
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
enum SmtpAuthConfig {
    Password(PasswordConfig),
}

#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
struct PasswordConfig {
    cmd: String,
}

With externally tagged enums

The following TOML string is properly parsed:

backend.imap.auth.password.cmd = "pass show password"

But this structure "allows" users to declare multiple backends, which is not possible:

backend.imap.auth.password.cmd = "pass show password"
backend.smtp.auth.password.cmd = "pass show password"
TOML parse error at line 2, column 1
  |
2 | backend.imap.auth.password.cmd = "pass show password"
  | ^^^^^^^
wanted exactly 1 element, more than 1 element

This representation is too permissive, it is too misleading for users and the error message is not explicit.

With internally tagged enums

By adding a #[serde(tag = "type")], the following string can be parsed:

backend.type = "imap"
backend.auth.type = "password"
backend.auth.cmd = "pass show password"

But the following wrong string gives a bad information about the error position: it always points to the first enum:

backend.type = "imap"
backend.auth.type = "wrong" # < wrong type
backend.auth.cmd = "pass show password"
TOML parse error at line 2, column 1
  |
2 | backend.type = "imap"
  | ^^^^^^^
unknown variant `wrong`, expected `password` or `oauth2`

This representation is perfect for users, and is the one I would like to keep. But the error line seems broken.

With adjacently tagged enums

By adding a #[serde(tag = "type", content = "content")], the following string can be parsed:

backend.type = "imap"
backend.content.auth.type = "password"
backend.content.auth.content.cmd = "pass show password"

And the previous mistake is now well reported:

backend.type = "imap"
backend.content.auth.type = "wrong"
backend.content.auth.content.cmd = "pass show password"
TOML parse error at line 3, column 29
  |
3 | backend.content.auth.type = "wrong"
  |                             ^^^^^^^
unknown variant `wrong`, expected `password` or `oauth2`

This representation reports well wrong information, but is way too verbose for users.


To summarize, I would like to keep the internally tagged enum representation but the reporting seems broken. Any idea what is going on?

@epage
Copy link
Member

epage commented Oct 25, 2024

This appears to be a duplicate of #535, closing in favor of that. If there is something I missed as to why this should be kept open, let us know!

@epage epage closed this as not planned Won't fix, can't repro, duplicate, stale Oct 25, 2024
@soywod
Copy link
Author

soywod commented Oct 25, 2024

So if I understand well the internally tagged and untagged share the same issue?

@epage
Copy link
Member

epage commented Oct 25, 2024

Yes

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 11, 2024
## [1.0.0] - 2024-12-09

The Himalaya CLI scope has changed. It does not include anymore the synchronization, nor the envelope watching. These scopes have moved to dedicated projects:

- [Neverest CLI](https://github.com/pimalaya/neverest), CLI to synchronize, backup and restore emails
- [Mirador CLI](https://github.com/pimalaya/mirador), CLI to watch mailbox changes

Due to the long time difference with the previous `v1.0.0-beta.4` release, this changelog may be incomplete. The simplest way to upgrade is to reconfigure Himalaya CLI from scratch, using the wizard or the [`config.sample.toml`](./config.sample.toml).

Himalaya CLI will now try to adopt the [conventional commits specification](https://github.com/conventional-commits/conventionalcommits.org). Tools like [`git-cliff`](https://git-cliff.org/) may help us generating more accurate changelogs in the future.

### Added

- Added `message edit` command to edit a message. To edit on place (replace a message), use `--on-place`.
- Added `account.list.table.preset` global config option, `accounts.<name>.folder.list.table.preset` and `accounts.<name>.envelope.list.table.preset` account config options.

  These options customize the shape of tables, see examples at [`comfy_table::presets`](https://docs.rs/comfy-table/latest/comfy_table/presets/index.html). Defaults to `"||  |-|||           "`, which corresponds to [`comfy_table::presets::ASCII_MARKDOWN`](https://docs.rs/comfy-table/latest/comfy_table/presets/constant.ASCII_MARKDOWN.html).

- Added `account.list.table.name-color` config option to customize the color used for the accounts' `NAME` column (defaults to `green`).
- Added `account.list.table.backends-color` config option to customize the color used for the folders' `BACKENDS` column (defaults to `blue`).
- Added `account.list.table.default-color` config option to customize the color used for the folders' `DEFAULT` column (defaults to `reset`).
- Added `accounts.<name>.folder.list.table.name-color` account config option to customize the color used for the folders' `NAME` column (defaults to `blue`).
- Added `accounts.<name>.folder.list.table.desc-color` account config option to customize the color used for the folders' `DESC` column (defaults to `green`).
- Added `accounts.<name>.envelope.list.table.id-color` account config option to customize the color used for the envelopes' `ID` column (defaults to `red`).
- Added `accounts.<name>.envelope.list.table.flags-color` account config option to customize the color used for the envelopes' `FLAGS` column (defaults to `reset`).
- Added `accounts.<name>.envelope.list.table.subject-color` account config option to customize the color used for the envelopes' `SUBJECT` column (defaults to `green`).
- Added `accounts.<name>.envelope.list.table.sender-color` account config option to customize the color used for the envelopes' `FROM` column (defaults to `blue`).
- Added `accounts.<name>.envelope.list.table.date-color` account config option to customize the color used for the envelopes' `DATE` column (defaults to `dark_yellow`).
- Added `accounts.<name>.envelope.list.table.unseen-char` account config option to customize the char used for unseen envelopes (defaults to `*`).
- Added `accounts.<name>.envelope.list.table.replied-char` account config option to customize the char used for replied envelopes (defaults to `R`).
- Added `accounts.<name>.envelope.list.table.flagged-char` account config option to customize the char used for flagged envelopes (defaults to `!`).
- Added `accounts.<name>.envelope.list.table.attachment-char` account config option to customize the char used for envelopes with at least one attachment (defaults to `@`).

### Changed

- Refactored the `account configure` command: this command stands now for creating or editing account configurations from the wizard. The command requires the `wizard` cargo feature.
- Improved the `account doctor` command: it now checks the state of the config, and the new `--fix` argument allows you to configure keyring, OAuth 2.0 etc.
- Improved long version `--version`. [#496]
- Improved error messages when missing cargo features. For example, if a TOML configuration uses the IMAP backend without the `imap` cargo features, the error `missing "imap" feature` is displayed. [#20](pimalaya/core#20)
- Normalized enum-based configurations, using the [internally tagged representation](https://serde.rs/enum-representations.html#internally-tagged) `type =`. It should reduce issues due to misconfiguration, and improve othe error messages. Yet it is not perfect, see [#802](toml-rs/toml#802):

  - `imap.*`, `maildir.*` and `notmuch.*` moved to `backend.*`:

	```toml
	# before
	imap.host = "localhost"
	imap.port = 143

	# after
	backend.type = "imap"
	backend.host = "localhost"
	backend.port = 143
	```

  - `smtp.*` and `sendmail.*` moved to `message.send.backend.*`:

	```toml
	# before
	smtp.host = "localhost"
	smtp.port = 25

	# after
	message.send.backend.type = "smtp"
	message.send.backend.host = "localhost"
	message.send.backend.port = 25
	```

  - `pgp.backend` renamed `pgp.type`:

	```toml
	# before
	pgp.backend = "commands"
	pgp.encrypt-cmd = "gpg --encrypt --quiet --armor <recipients>"

	# after
	pgp.type = "commands"
	pgp.encrypt-cmd = "gpg --encrypt --quiet --armor <recipients>"
	```

  - `{imap,smtp}.auth` moved as well:

    ```toml
    # before
    imap.password.cmd = "pass show example"
    smtp.oauth2.method = "xoauth2"

    # after
    backend.auth.type = "password"
    backend.auth.cmd = "pass show example"
    message.send.backend.auth.type = "oauth2"
    message.send.backend.auth.method = "xoauth2"
    ```

- Moved IMAP and SMTP `encryption` to `encryption.type`.

  This change prepares the config to accept different TLS providers with their options. The `true` and `false` variant have been removed as well:

	```toml
	# before
	backend.encryption = "none" # or false
	backend.encryption = "start-tls"
	message.send.backend.encryption = "tls" # or true

	# after
	backend.encryption.type = "none"
	backend.encryption.type = "start-tls"
	message.send.backend.encryption.type = "tls"
	```

### Fixed

- Fixed pre-release archives issue. [#492]
- Fixed mailto parsing issue. [core#10]
- Fixed `Answered` flag not set when replying to a message. [#508]

### Removed

- Removed systemd service from `assets/` folder, as Himalaya CLI scope does not include synchronization nor watching anymore.

## [1.0.0-beta.4] - 2024-04-16

### Added

- Added systemd service in `assets/` folder.
- Added configuration option `message.delete.style` that can be either `folder` (deleted messages are moved to the Trash folder, default style) or `flag` (deleted messages receive the Deleted flag).
- Added `--debug` as an alias for `RUST_LOG=debug`.
- Added `--trace` as an alias for `RUST_LOG=trace` and `RUST_BACKTRACE=1`.
- Added notes about `--debug` and `--trace` when error occurs.

### Changed

- **Added back the search feature**: you can now give an optional filter and sort query at the end of the `envelope list` command. See `envelope list --help` or [pimalaya.org](https://pimalaya.org/himalaya/cli/master/usage/advanced/envelope/list.html#query) for more detail on the search API.
- Changed the `envelope list` folder argument due to the search query: it became a flag `--folder|-f`.
- Made the global `--config|-c` option repeatable: the first option is considered the path to the main config, and successive options are considered partial overrides [#184].
- Refactored error management: error should be more clear, colored and can now contain spantrace and backtrace.
- Made `--help` content wrapping properly thanks to the `clap` cargo feature `wrap_help`.
- Improved `template {new,reply,forward}` command JSON output: they return now a JSON object with 3 properties:
  - `content`: the content of the template
  - `cursor.row`: the row at which the cursor should be placed by the interface using the template
  - `cursor.col`: the column at which the cursor should be placed by the interface using the template

### Fixed

- Fixed watch IMAP envelopes when folder was empty [#179].
- Prevented parsing of undefined config options [#188].
- Fixed `In-Reply-To` header being skipped from mailto URLs [#194].
- Fixed error page out of bounds when filtering envelopes returned an empty result [#195].

## [1.0.0-beta.3] - 2024-02-25

### Added

- Added `account check-up` command.
- Added wizard warning about google passwords [#41].

### Changed

- Removed account configurations flatten level in order to improve diagnostic errors, due to a [bug](toml-rs/toml#589 (comment)) in clap. **This means that accounts need to be prefixed by `accounts`: `[my-account]` becomes `[accounts.my-account]`**. It also opens doors for interface-specific configurations.
- Rolled back cargo feature additions from the previous release. It was a mistake: the amount of features was too big, the code (both CLI and lib) was too hard to maintain. Cargo features kept: `imap`, `maildir`, `notmuch`, `smtp`, `sendmail`, `account-sync`, `account-discovery`, `pgp-gpg`, `pgp-commands` and `pgp-native`.
- Moved `sync.strategy` to `folder.sync.filter`.
- Changed location of the synchronization data from `$XDG_DATA_HOME/himalaya/<account-name>` to `$XDG_DATA_HOME/pimalaya/email/sync/<account-name>-cache`.
- Changed location of the synchronization cache from `sync.dir` to `$XDG_CACHE_HOME/pimalaya/email/sync/<hash>/`.
- Replaced id mapping database `SQLite` by `sled`, a pure key-val store written in Rust to improve portability of the tool. **Therefore, id aliases are reset**.
- Improved pre and post edit choices interaction [#58].
- Improved account synchronization performances, making it 50% faster than `mbsync` and 370% faster than `OfflineIMAP`.
- Changed `envelope.watch.{event}.{hook}`: hooks can now be cumulated. For example it is possible to send a system notification and execute a shell command when receiving a new envelope:

  ```toml
  envelope.watch.received.notify.summary = "New message from {sender}"
  envelope.watch.received.notify.body = "{subject}"
  envelope.watch.received.cmd = "echo {id} >> /tmp/new-email-counter"
  ```

### Fixed

- Fixed bug that was preventing watch placeholders to be replaced when using shell command hook.
- Fixed watch IMAP envelopes issue preventing events to be triggered.
- Fixed DNS account discovery priority issues.
- Fixed SMTP messages not properly sent to all recipients [#172].
- Fixed backend feature badly linked, leading to reply and forward message errors [#173].

## [1.0.0-beta.2] - 2024-01-27

### Added

- Added cargo feature `wizard`, enabled by default.
- Added one cargo feature per backend feature:
  - `account` including `account-configure`, `account-list`, `account-sync` and the `account-subcmd`
  - `folder` including `folder-add`, `folder-list`, `folder-expunge`, `folder-purge`, `folder-delete` and the `folder-subcmd`
  - `envelope` including `envelope-list`, `envelope-watch`, `envelope-get` and the `envelope-subcmd`
  - `flag` including `flag-add`, `flag-set`, `flag-remove` and the `flag-subcmd`
  - `message` including `message-read`, `message-write`, `message-mailto`, `message-reply`, `message-forward`, `message-copy`, `message-move`, `message-delete`, `message-save`, `message-send` and the `message-subcmd`
  - `attachment` including `attachment-download` and the `attachment-subcmd`
  - `template` including `template-write`, `template-reply`, `template-forward`, `template-save`, `template-send` and the `template-subcmd`
- Added wizard capability to autodetect IMAP and SMTP configurations, based on the [Thunderbird Autoconfiguration](https://wiki.mozilla.org/Thunderbird:Autoconfiguration) standard.
- Added back Notmuch backend features.

### Changed

- Renamed `folder create` to `folder add` in order to better match types. An alias has been set up, so both `create` and `add` still work.

### Fixed

- Fixed default command: running `himalaya` without argument lists envelopes, as it used to be in previous versions.
- Fixed bug when listing envelopes with `backend = "imap"`, `sync.enable = true` and `envelope.watch.backend = "imap"` led to unwanted IMAP connection creation (which slowed down the listing).
- Fixed builds related to enabled cargo features.

## [1.0.0-beta] - 2024-01-01

Few major concepts changed:

- The concept of *Backend* and *Sender* changed. The Sender does not exist anymore (it is now a backend feature). A Backend is now a set of features like add folders, list envelopes or send raw message. The backend of every single feature can be customized in the configuration file, which gives users more flexibility. Here the list of backend features that can be customized:
  - `backend` ***(required)***: the backend used by default by all backend features (`maildir`, `imap` or `notmuch`)
  - `folder.add.backend`: override the backend used for creating folders (`maildir`, `imap` or `notmuch`)
  - `folder.list.backend`: override the backend used for listing folders (`maildir`, `imap` or `notmuch`)
  - `folder.expunge.backend`: override the backend used for expunging folders (`maildir`, `imap` or `notmuch`)
  - `folder.purge.backend`: override the backend used for purging folders (`maildir`, `imap` or `notmuch`)
  - `folder.delete.backend`: override the backend used for deleting folders (`maildir`, `imap` or `notmuch`)
  - `envelope.list.backend`: override the backend used for listing envelopes (`maildir`, `imap` or `notmuch`)
  - `envelope.get.backend`: override the backend used for getting envelopes (`maildir`, `imap` or `notmuch`)
  - `envelope.watch.backend`: override the backend used for watching envelopes (`maildir`, `imap` or `notmuch`)
  - `flag.add.backend`: override the backend used for adding flags (`maildir`, `imap` or `notmuch`)
  - `flag.set.backend`: override the backend used for setting flags (`maildir`, `imap` or `notmuch`)
  - `flag.remove.backend`: override the backend used for removing flags (`maildir`, `imap` or `notmuch`)
  - `message.send.backend` ***(required)***: override the backend used for sending messages (`sendmail` or `smtp`)
  - `message.read.backend`: override the backend used for reading messages (`maildir`, `imap` or `notmuch`)
  - `message.write.backend`: override the backend used for adding flags (`maildir`, `imap` or `notmuch`)
  - `message.copy.backend`: override the backend used for copying messages (`maildir`, `imap` or `notmuch`)
  - `message.move.backend`: override the backend used for moving messages (`maildir`, `imap` or `notmuch`)
- The CLI API changed: every command is now prefixed by its domain following the format `himalaya <domain> <action>`. List of domain available by running `himalaya -h` and list of actions for a domain by running `himalaya <domain> -h`.
- TOML configuration file options use now the dot notation rather than the dash notation. For example, `folder-listing-page-size` became `folder.list.page-size`. See the [changed](#changed) section below for more details.

### Added

- Added cargo feature `maildir` (not plugged yet).
- Added cargo feature `sendmail` (not plugged yet).
- Added watch hooks `envelope.watch.received` (when a new envelope is received) and `envelope.watch.any` (for any other event related to envelopes). A watch hook can be:
  - A shell command: `envelope.watch.any.cmd = "mbsync -a"`
  - A system notification:
    - `envelope.watch.received.notify.summary = "📬 New message from {sender}"`: customize the notification summary (title)
    - `envelope.watch.received.notify.body = "{subject}"`: customize the notification body (content)

	*Available placeholders: id, subject, sender, sender.name, sender.address, recipient, recipient.name, recipient.address.*
- Added watch support for Maildir backend features.

### Changed

- Renamed cargo feature `imap-backend` → `imap`.
- Renamed cargo feature `notmuch-backend` → `notmuch`.
- Renamed cargo feature `smtp-sender` → `smtp`.
- Changed the goal of the config option `backend`: it is now the default backend used for all backend features. Valid backends: `imap`, `maildir`, `notmuch`.
- Moved `folder-aliases` config option to `folder.alias(es)`.
- Moved `folder-listing-page-size` config option to `folder.list.page-size`.
- Moved `email-listing-page-size` config option to `envelope.list.page-size`.
- Moved `email-listing-datetime-fmt` config option to `envelope.list.datetime-fmt`.
- Moved `email-listing-datetime-local-tz` config option to `envelope.list.datetime-local-tz`.
- Moved `email-reading-headers` config option to `message.read.headers`.
- Moved `email-reading-format` config option to `message.read.format`.
- Moved `email-writing-headers` config option to `message.write.headers`.
- Move `email-sending-save-copy` config option to `message.send.save-copy`.
- Move `email-hooks.pre-send` config option to `message.send.pre-hook`.
- Moved `sync` config option to `sync.enable`.
- Moved `sync-dir` config option to `sync.dir`.
- Moved `sync-folders-strategy` config option to `sync.strategy`.
- Moved `maildir-*` config options to `maildir.*`.
- Moved `imap-*` config options to `imap.*`.
- Moved `notmuch-*` config options to `notmuch.*`.
- Moved `sendmail-*` config options to `sendmail.*`.
- Moved `smtp-*` config options to `smtp.*`.
- Replaced options `imap-ssl`, `imap-starttls` and `imap-insecure` by `imap.encryption`:
  - `imap.encryption = "tls" | true`: use required encryption (SSL/TLS)
  - `imap.encryption = "start-tls"`: use opportunistic encryption (StartTLS)
  - `imap.encryption = "none" | false`: do not use any encryption
- Replaced options `smtp-ssl`, `smtp-starttls` and `smtp-insecure` by `smtp.encryption`:
  - `smtp.encryption = "tls" | true`: use required encryption (SSL/TLS)
  - `smtp.encryption = "start-tls"`: use opportunistic encryption (StartTLS)
  - `smtp.encryption = "none" | false`: do not use any encryption

### Removed

- Disabled temporarily the `notmuch` backend because it needs to be refactored using the backend features system (it should be reimplemented soon).
- Disabled temporarily the `search` and `sort` command because they need to be refactored, see [#39].
- Removed the `notify` command (replaced by the new `watch` command).
- Removed all global options except for `display-name`, `signature`, `signature-delim` and `downloads-dir`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants