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

Preserve the comments of the "cli.toml" on changes #2002

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mamcx
Copy link
Contributor

@mamcx mamcx commented Nov 19, 2024

Description of Changes

We would like to preserve a user's manual edits, including comments, so now 'tolm_edit' is used for mutating the config file.

Closes #1798.

API and ABI breaking changes

Marked breaking because of the change of crate to parse and save. Is the low-level used by 'toml'.

Expected complexity level and risk

2:

Marked breaking because of the change of how to parse and save the file (that mutates instead of generating a fresh immutable view of the config) even if according to test there is none.

Testing

  • Write unit testing that simulates different edits.
  • Run 'standalone' and execute several 'cli' commands to see manually if the config was updated without issues.

@mamcx mamcx added abi-break A PR that makes an ABI breaking change release-1.0 labels Nov 19, 2024
@mamcx mamcx self-assigned this Nov 19, 2024
host,
protocol,
ecdsa_public_key,
}
Copy link
Collaborator

@bfops bfops Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should we have a ServerConfig::from_table? (or an impl From<&Table> for ServerConfig or something)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    pub fn from_table:(table: &Table) -> Self {
        let nickname = table.get("nickname").and_then(Item::as_str).map(String::from);
        let host = table.get("host").and_then(Item::as_str).map(String::from).unwrap();
        let protocol = table.get("protocol").and_then(Item::as_str).map(String::from).unwrap();
        let ecdsa_public_key = table.get("ecdsa_public_key").and_then(Item::as_str).map(String::from);
        ServerConfig {
            nickname,
            host,
            protocol,
            ecdsa_public_key,
        }
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// Returns a preserving copy of [Config].
fn doc(&self) -> DocumentMut {
let mut doc = self.doc.clone();

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we do something like let RawConfig { ... } = home so people don't forget to update this function if they add more fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mamcx mamcx force-pushed the mamcx/config-preserve-edits branch from 82ba41f to d69605d Compare December 2, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abi-break A PR that makes an ABI breaking change release-1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI - Preserve comments when mutating the config file
2 participants