-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: associate with Dockerfile.* #18
base: main
Are you sure you want to change the base?
Conversation
This convention used to be noted in the Docker documentation for some time recently, and has stuck here and there even though the docs no longer refer to it. docker/docs#9245 (comment) Closes dprint#6
00cbb5e
to
bf3f1ef
Compare
@@ -32,7 +32,7 @@ impl SyncPluginHandler<Configuration> for DockerfilePluginHandler { | |||
}, | |||
file_matching: FileMatchingInfo { | |||
file_extensions: vec!["dockerfile".to_string()], | |||
file_names: vec!["Dockerfile".to_string()], | |||
file_names: vec!["Dockerfile".to_string(), "Dockerfile.*".to_string()], |
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.
This is arguably a bit of a hack/abuse (I have a feeling that file_names
was not intended to be used with globs in the entries), but looking at dprint main sources, I think it would work. Setting up file_prefixes
for something like this would sound a bit overkill to me.
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.
Yeah, it won't work. This doesn't do pattern matching.
I think the only way to make this work in dprint is for someone to manually add "associations" (https://dprint.dev/config/#associations) under their dockerfile config.
Overall, I'm not too interested in suporting the dockerfile.*
convention out of the box because it's a bad convention and it's good docker has switched over to using a file extension.
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.
For reference, I think this will work today:
{
"dockerfile": {
"associations": [
"**/Dockerfile.*",
"**/Dockerfile",
"**/*.dockerfile"
]
},
"plugins": [
"https://plugins.dprint.dev/dockerfile-0.3.2.wasm"
]
}
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.
The reason I think it could work is that file_names
entries seem to be just concatenated to **/
and it doesn't seem there's anything magical whether the entries are globs or plain filenames: https://github.com/dprint/dprint/blob/14eb074e9baf8d0533f04ad954e17a5a375e6385/crates/dprint/src/paths.rs#L161
Using "associations" works, I'm using that currently, but it's a chore that one has to repeat all the built in globs in it. I find that surprising because it seems they would be appended to the existing builtin globs: https://github.com/dprint/dprint/blob/14eb074e9baf8d0533f04ad954e17a5a375e6385/crates/dprint/src/paths.rs#L163-L164
Alas, if I define
{
"dockerfile": {
"associations": [
"Dockerfile.*"
]
[...]
}
...then only Dockerfile.*
match. Need to add Dockerfile
and *.dockerfile
there to get them back.
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.
Oh, yeah, maybe it will work. You can try building by calling:
run: cargo build --target wasm32-unknown-unknown --features wasm --release |
Then the wasm file will be somewhere in the target
folder (just import the file relatively from your dprint.config file)
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.
Hmph, doesn't seem to work for some reason after all :/
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.
It expands to this glob:
**/{Dockerfile,Dockerfile.*}
I guess that doesn't work. Maybe the dprint cli could be updated to support some basic wildcards in filenames to make it work.
This convention used to be noted in the Docker documentation for some time recently, and has stuck here and there even though the docs no longer refer to it.
docker/docs#9245 (comment)
Closes #6
Warning: untested monkey patch.