adding global variable to enable symlinks by default if type not specified

[ducks]

howdy, I am interested in this feature as well, #169, and took a shot at implementing it. I am not very practiced in rust lately so I just wanted to make sure I could at least get something that worked before getting caught up in any specifics like where to store the config or what to name it.

This PR adds a variables field to the GlobalConfig. Then, when merging and processing the config files, it checks for a field, enable_symlink_by_default, and if that is true, it converts any packages with any files that are FileTarget::Automatic to FileTarget::Symbolic. I thought the deployer shouldn't really care about the FileTarget type and decided to handle this conversion in the config. I also added a test to show it converts FileTarget::Automatic package files but not a specified template.

Please let me know your thoughts and feedback, thanks.

[SuperCuber]

Looks good, and a settings key in global.toml is something I've been putting off for no good reason. It's a good idea to get it started, and I think we can figure out local/include.toml overrides for settings if we ever want 🤷

Another point is we need to remember to document this in the wiki

[ducks]

Looks good, and a settings key in global.toml is something I've been putting off for no good reason. It's a good idea to get it started, and I think we can figure out local/include.toml overrides for settings if we ever want 🤷

Another point is we need to remember to document this in the wiki

thanks so much for the comments. I will be working on these updates and also adding some documentation.

also, I was rereading the issue and I was wondering if it might be worth making this more flexible. there could be a settings field called "default_target_type" and have symlink or template as options.

[SuperCuber]

also, I was rereading the issue and I was wondering if it might be worth making this more flexible. there could be a settings field called "default_target_type" and have symlink or template as options.

Sounds like a good idea. Have it be automatic / symlink / template, with automatic being the default. Then you can do something like

#[derive(Deserialize, Default)]
#[serde(rename_all = "lowercase")]
enum DefaultTargetType {
    Symlink,
    Template,
    #[default]
    Automatic,
}

#[derive(Deserialize)]
struct Settings {
    #[serde(default)]
    default_target_type: DefaultTargetType,
}

[ducks]

pushed up some changes:

• added a settings field to the GlobalConfig which includes the default_target_type field
• based on the property, transform any package files that aren't explicity set
• added a couple more tests to cover each option

I also wrote some simple documentation below. I'm thinking it could go right after the variables section and before the complex templates section on the "editing setup and configuration" page.

Settings

Dotter allows you to configure some global settings.

There is currently only one setting, but if you have an idea, please open an issue.

default_target_type
options: symbolic, template, or automatic
defaults to automatic

Sets the default target type for any package files that don't explicity set a target type.

example:

[settings]
default_target_type = "symbolic"

[nvim]
depends = []

[nvim.files]
nvim = "~/.config/nvim"

[SuperCuber]

Looking good so far :D

[SuperCuber]

IMO you overthought this a little with the FileTargetTransform. How about

for value in files.values_mut() {
    *value = match *value {
        FileTarget::Automatic(target) if global.settings.default_target_type == DefaultTargetType::Symbolic => {
            FileTarget::Symbolic(SymbolicTarget::from(target))
        },
        FileTarget::Automatic(target) if global.settings.default_target_type == DefaultTargetType::Template => {
            FileTarget::ComplexTemplate(TemplateTarget::from(target))
        },
        other => other,
    }
}

Or maybe

for value in files.values_mut() {
    if let FileTarget::Automatic(target) = *value {
        match global.settings.default_target_type {
            DefaultTargetType::Symbolic => {
                *value = FileTarget::Symbolic(SymbolicTarget::from(target));
            },
            DefaultTargetType::Template => {
                *value = FileTarget::ComplexTemplate(TemplateTarget::from(target));
            },
            _ => {},
        }
    }
}

Both of those might run into borrowing issues so you might have to use .into_iter().map 🙃
The block could be extracted into a function that takes default_target_type if you think it would be cleaner

[ducks]

yeah, I definitely agree re overthinking it. I like either of those options much better and will be working on implementing one. thanks.

[SuperCuber]

Also there's some failing CI.
For the formatter you can run cargo fmt
For clippy, you can put #[allow(dead_code)] on the field it's complaining about. It makes sense to add the settings to the Configuration even if it's unused for now.

[SuperCuber]

Still need to add the allow(dead_code), see CI error from Clippy

Looks good!

[ducks]

huh, weird. I ran cargo clippy locally and it didn't report anything.

either way, I added the #[allow(dead_code)] to that field.

[SuperCuber]

🎉 Thank you for the contribution!