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

Error spans incorrect when using flatten #589

Open
wez opened this issue Jul 28, 2023 · 6 comments
Open

Error spans incorrect when using flatten #589

wez opened this issue Jul 28, 2023 · 6 comments
Labels
A-error Area: Error reporting A-serde Area: Serde integration C-bug Category: Things not working as expected

Comments

@wez
Copy link

wez commented Jul 28, 2023

Here's a link to my example/repro:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=5651e21bcb73e6fabf7e1d0ef67660d2

and inline for convenience:

use serde::Deserialize;
use std::collections::HashMap;

#[derive(Deserialize, Debug)]
struct Thing {
   #[serde(flatten)]
   entries: HashMap<String, Entry>,
}

#[derive(Deserialize, Debug)]
struct Entry {
   foo: u32,
}

fn main() {
    let data = "# comment line\n[\"default\"]\nfooo = 42";
    match toml::from_str::<Thing>(&data) {
        Ok(data) => println!("{data:#?}"),
        Err(err) => println!("{err:#}"),
    }
}

produces:

TOML parse error at line 1, column 1
  |
1 | # comment line
  | ^
missing field `foo`

I would expect the line number to be 3 rather than 1, and for the context in the error message to refer to the appropriate part of the file.

@epage
Copy link
Member

epage commented Jul 28, 2023

Without debugging this, my best guess is that something in the way our code or serde is structured is preventing us from associating the foo error with the default table, making us associate it with the whole document which puts the error at the beginning of the file.

@epage epage added C-bug Category: Things not working as expected A-error Area: Error reporting labels Jul 28, 2023
@soywod
Copy link

soywod commented Dec 29, 2023

I have the same issue with a #[serde(flatten)] on a HashMap as well, which is a real problem for my tool because any wrong option points to the whole document:

#[derive(Clone, Debug, Default, Eq, PartialEq, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub struct TomlConfig {
    #[serde(alias = "name")]
    pub display_name: Option<String>,
    pub signature: Option<String>,
    pub signature_delim: Option<String>,
    pub downloads_dir: Option<PathBuf>,

    #[serde(flatten)]
    pub accounts: HashMap<String, TomlAccountConfig>,
}
[example]
backend = "imapp"
Error: cannot parse config file at "~/code/himalaya/config.sample.toml"

Caused by:
    TOML parse error at line 1, column 1
      |
    1 | [example]
      | ^
    unknown variant `imapp`, expected one of `maildir`, `imap`, `smtp`, `sendmail`

@baszalmstra
Copy link

I ran into this as well. I think it has something to do with flatten. See https://www.rustexplorer.com/b/uspq0k for a minimal reproducible case.

@epage
Copy link
Member

epage commented Jan 6, 2024

If I'm reading the serde_derive expanded code, it looks like this is a problem with how serde_derive deserializes things.

serde_derive seems to flatten content into something like serde_value (which can't error) and then deserializes into the flattened type (can error). This means the error is coming out of the deserializer for the outer table, and not on the individual field, and that is all toml/toml_edit can rely on for associating it with the code.

@soywod
Copy link

soywod commented Jan 6, 2024

Indeed, looks like a know issue at serde_derive, see serde-rs/serde#2581 and serde-rs/serde#1183. Should we close this issue or wait for serde_derive to fix it before closing it?

@epage
Copy link
Member

epage commented Jan 6, 2024

I think keeping it open but blocked on serde will better help people discover it.

@epage epage changed the title Line numbers and context are incorrect when data contains comments Error spans incorrect when using flatten Mar 8, 2024
@epage epage added the A-serde Area: Serde integration label Mar 8, 2024
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
A-error Area: Error reporting A-serde Area: Serde integration C-bug Category: Things not working as expected
Projects
None yet
Development

No branches or pull requests

4 participants