https://pantsbuild.org/ logo
Join Slack
Powered by
Is `ConfigFilesRequest` the canonical mechanism to...
# plugins
w
Is 
ConfigFilesRequest
the canonical mechanism to grab a config file from a source root? Like a 
.clang-format
or 
.eslint.rc
or similar? If so, is there any formal documentation around it outside of source code?
I've been using 
isort
and 
black
as inspiration, so I thought I understood how it works, but I'm less certain. When I use the 
specified
field and point directly at my 
.clang-format
file, it works great, no problems. When I use the 
check_existence
(which gets passed into 
PathGlobs
- I'm a bit less sure. In my case, 
.clang-format
, a few variations, and nothing specified are all valid cases. If I use 
specified
- not placing any formatter files in the firectory causes the glob to fail, even though no config file is a valid workflow. If I place all valid config file permutations into 
check-existence
, the 
fmt
runs, but it doesn't pull and use in my 
.clang-format
file. I feel like I'm missing something trivial here
Whatever I do here will be directly pulled into the NodeJS Prettier and ESLint cases, as they also support multiple possible config files by default (and I think Prettier also supports no config files)
b
I think Isort uses hierarchical configs, so I'd focus on that more?
h
There is no formal documentation of it other than the several examples of using it (and it does support hierarchical configs)
w
Not sure if the hierarchical comes into play - just one of any number of options, where no file is a valid option. From everything I'm doing and seeing in my logs, it 'should' work, so I feel as though I might have some critical misunderstanding of this API. For example, I think leaving 
specified
empty is perfectly valid, and then only using 
check_existence
- at which point, we should be good. But it almost looks like 
check_existence
wasn't putting anything into my 
Digest
which is 🤯 I'll be digging into it again in just a few minutes, so hopefully I was just doing something flagrantly wrong last night and it'll "just work ™️ "
h
Yeah, IIRC 
specified
means "this file must exist" and 
check_existence
is for a set of files that may or may not exist, but if they do, pull them in
And then there is "pull them in only if they contain this snippet"
So 
check_existence
is what you want
if it's not working then let's debug that!
w
Yep, the 
check_content
was great, it opens up a few possibilities for Prettier (specifically, how I use prettier by keeping config in the 
package.json
) I'll report back shortly if I can get this working. Judging by the other code, and these responses, I'm not missing anything - so hopefully it's just a typo or something 🙂
h
the paths are relative to the buildroot, FYI
w
Found that out the hard way 🙂
Ugh, works on one computer, but not the other. That's like.... the worst possible outcome
h
Uuuuuuuuuurgh
the worst
w
[sets computer on fire] Okay, turns out there are no more bugs
I will say though, this is getting excessive
😂 1
f
Re alternate config file examples, 
ScalafmtConfigFiles
is an example that doesn't use 
ConfigFilesRequest
👍 1
h
I didn't know about that one
why so?
f
We support putting a 
.scalafmt.conf
in subdirectories to take effect for Scala files in that subtree.
(Scalafmt does not know how to merge config files.)
h
And 
ConfigFilesRequest
can't handle that via a glob?
f
well it’s more nuanced than that. If you use 
specified
with 
ConfigFilesRequest
, then you must know the paths in advance and it is an error if they don’t exist (and wants the glob to come from an option name i.e. 
request.specified_option_name
). If done via 
request.discovery
, then you have to specify 
check_content
which also does not work with 
.scalafmt.conf
, since no specific snippet to check for. So ultimately easier for implementing the algorithm to just construct the globs it needs based on Scala source directories and just call 
PathGlobs
. Most of the magic is in matching source directories to the nearest ancestor directory with a 
.scalafmt.conf
h
then you have to specify check_content
You can set it to empty, you only have to use one of 
check_content
or 
check_exist
I think using 
ConfigFiles
would work, I honestly only didn't think of that in code review
And yeah SJ, to reiterate, it's not required to use 
ConfigFiles
. I wrote it when we added config file discovery in Pants 2.4(?) to DRY (There was a dark age where you had to explicitly specify every config file)
w
Thanks @hundreds-father-404 - I have it working rather well, I'm just trying to figure out the best path forward while being mindful of 
files
,
resources
, other source files, and then configuration. I think this seems like the most reasonable mechanism, or configuration would also need to be in the source files glob - which I'm not sure I like
👍 1
h
yeah that's why we did config file support as options in the first place, seemed overkill and confusing to have a dedicated target. Of course, if you want, nothing is stopping someone from also adding the config to a 
file
or 
resource
target
w
Alright, it's worth noting for my own sake that this was something I was confused about while building these plugins. I assumed files/resources was the correct way, then I assumed bundling in the language source files, then I saw PathGlobs in a rule because I saw that in the docs (I think under the common linter example), then finally I went with ConfigFilesRequest because it eventually seemed like the least amount of effort.
👍 1
5 Views
Open in Slack
PreviousNext
Powered by