search

mitranim / sublime-rust-fmt

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

Add auto option for legacy_write_mode_option #8

Closed dbeckwith closed 6 years ago

dbeckwith commented 6 years ago

Fixes #7

Adds an auto option for legacy_write_mode_option that detects if option is needed based on rustfmt version.

mitranim commented 6 years ago

Very nice of you to provide a solution. I hope you don't mind some counter-suggestions 🙂

Autodetection is fantastic in principle, but I'm wary of adding a mandatory subprocess call. It adds code, latency, and brittleness. Are you sure this can't be solved by using Sublime's project-specific settings? It's slightly more annoying to setup, but in the long run, everyone should update from the legacy version, and we won't need this overhead.

If we do use version detection, the regexp should probably be truncated to r'(\d+)\.(\d+)\.(\d+)' to make it more flexible towards future version strings.

dbeckwith commented 6 years ago

Yes that's a good point about the extra subprocess call. Project-specific settings are probably a better solution, since once you upgrade past 0.8.0, the flag is pretty much unneeded, and won't change very often anyway. Probably still a good idea to add some documentation to legacy_write_mode_option to mention which versions it's needed for (< 0.8.0). I'll close this and #7, thanks for the help and feedback!