mitranim / sublime-rust-fmt

Sublime Text plugin that formats Rust code with rustfmt
30 stars 3 forks source link

Updates plugin to check for rustfmt config file #5

Closed csmulhern closed 6 years ago

csmulhern commented 6 years ago

This pull request extends the plugin to look for a rustfmt.toml or .rustfmt.toml config file on the path from the current file to the root directory, so that project specific settings can be respected.

mitranim commented 6 years ago

Thanks for the contribution. Are you sure we need to look for the TOML files? rustfmt will supposedly search for it by itself, starting in the --config-path folder and going up. We could pass the folder and avoid doing the walk and even hardcoding the config name. Unfortunately I'm unable to test this idea as rustfmt-nightly doesn't run on my system at the moment.

mitranim commented 6 years ago

Versions of rustfmt that don't support --config-path (at least some of them) exit with an error when you pass this option. It needs to be opt-in. I would add a boolean setting with default false and check for it before appending config path to args:

{
  "format_on_save": true,
  // Can be a string (path) or a list (executable + args).
  "executable": "rustfmt",
  // Whether to tell `rustfmt` to search for the TOML config in the current
  // folder. For anonymous buffers, attempts to guess the current folder from
  // the window. Off by default because at the time of writing, the default
  // version of `rustfmt` doesn't support configs.
  "use_config_path": false
}
csmulhern commented 6 years ago

No problem!

Unfortunately, there are a couple of problems with just passing the file's directory to rustfmt via --config-path.

  1. Unlike the documentation suggests, --config-path just checks the directory of the provided path, it does not do a recursive search.

See: https://github.com/rust-lang-nursery/rustfmt/blob/master/src/bin/rustfmt.rs#L436 https://github.com/rust-lang-nursery/rustfmt/blob/master/src/config.rs#L468

  1. If a path is passed to --config-path, and no rustfmt configuration file is found, the program exits with an error, it does not fallback to using a default configuration.

See: https://github.com/rust-lang-nursery/rustfmt/blob/master/src/bin/rustfmt.rs#L433.

csmulhern commented 6 years ago

I've added the use_config_path setting as suggested.

mitranim commented 6 years ago

Looking good!

Would only make a minor change: append_config_path uses an obscure Python feature where += mutates a list argument. Most languages and even most Python types don't work this way. Let's follow the principle of least astonishment:

def find_config_path(path):
    return first(map(config_for_dir, walk_to_root(path)), lambda x: x is not None)

def run_format(view, input, encoding):
    args = to_list(settings_get(view, 'executable')) + ['--write-mode', 'display']

    if settings_get(view, 'use_config_path'):
        config = find_config_path(view.file_name())
        if config:
            args += ['--config-path', config]
csmulhern commented 6 years ago

Sounds good to me.

Done.

mitranim commented 6 years ago

Nice! Thanks for the work, it's ready to merge.

Snap, just remembered about anonymous buffers. The plugin should work on them seamlessly. They don't have paths, but we could guess from the current Sublime window. This could be a decent first approximation:

path = view.file_name() or first(view.window().folders(), bool)
config = path and find_config_path(path)

Would be nice if you could test and add this.

csmulhern commented 6 years ago

Cool.

I modified the code as you suggested, to provide support for anonymous buffers, and things are working as expected. Thanks for the help.

mitranim commented 6 years ago

Nice! Released 0.1.5 with support for config files. Thank you for the contribution. 😄