-
Notifications
You must be signed in to change notification settings - Fork 55
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
Apply exclude_files
to files
absolute paths
#356
Apply exclude_files
to files
absolute paths
#356
Conversation
When calling `erlfmt` with absolute paths for the files to format, the excludes don't apply, even if the resolved paths of the excludes would match the files to format. Resolve the paths of the excludes based on the current working directory and add them to the excludes. This issue was observed when using the [VS Code Erlang Formatter extension](https://marketplace.visualstudio.com/items?itemName=szTheory.erlang-formatter). The extension invokes `rebar3 fmt` with the full path of the file to format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, what do you think @michalmuskala ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One concern about windows support.
The second concerns would be with symlinks. I'm not sure it would all work correctly in this case.
Wouldn't an alternative of rather than checking for file equality to check for prefixes work?
This handles Windows' `volumerelative` path types by properly resolving them based on the "per-drive" working directory. Also, resolve the input file absolute paths. This is new functionality; if you were previously to use a file path like `a/../b`, that would have been directly matched against the excludes. As a result, an exclude of `b` would not have applied. This change makes sure relative path resolution is consistent between `files`/`exclude_files`. To avoid displaying different paths from what was requesting by config or CLI options (e.g. when using the verbose flag), use a map of absolute paths -> original paths and return the original paths from `erlfmt_cli:set_difference/2` after applying excludes.
@awalterschulze @michalmuskala ready for further review 🙂 I've addressed Windows support by switching to using The change also makes sure I intentionally avoided any symlink resolution. The existing behaviour of erlfmt treats symlinks as distinct files. e.g.
I think this makes sense and is consistent with most treatment of symlinks in my experience. One example is the script name argument when running a bash script:
Unless the script explicitly follows its script name symlink, it thinks it is a unique script. |
Thank you! ❤️ |
When calling
erlfmt
with absolute paths for the files to format, the excludes don't apply, even if the resolved paths of the excludes would match the files to format.Resolve the paths of the excludes based on the current working directory and add them to the excludes.
This issue was observed when using the
VS Code Erlang Formatter extension. The extension invokes
rebar3 fmt
with the full path of the file to format.