identify icon indicating copy to clipboard operation
identify copied to clipboard

Identify buf-related files

Open jalaziz opened this issue 1 year ago • 3 comments

Tag buf.yaml, buf.gen.yaml, and buf.lock files with buf and buf-lock tags respectively.

Note: This doesn't identify custom gen files of the form buf.gen.foo.yaml as buf.

jalaziz avatar May 13 '24 08:05 jalaziz

These would be pretty useful when working with protobuf files and buf.

The one issue is that you may have multiple "gen" files, with slightly different extensions. They could be easily supported by simply registering buf or buf.gen, but that seems like more of a hack than a fix. Maybe buf.gen makes sense, but not too sure about buf.

I also didn't include the buf.work.yaml workspace file since it's unclear if there's a need to perform pre-commit steps when that file changes.

More info can be found here.

jalaziz avatar May 13 '24 08:05 jalaziz

is there a reason to add these? filtering with files should work fine for these and I don't think types gets you anything here

asottile avatar May 29 '24 14:05 asottile

Similar reason to: https://github.com/pre-commit/identify/pull/459#issuecomment-2138489963.

It's needed to avoid the limitation of having to solely use files (due to the inability to OR across both) and losing all the benefits that types has to offer.

The buf pre-commit hooks already use the proto type. Without this change, you end up having to use files exclusively to match against the buf files AND the proto files. Which also means you have to explicitly override types. All doable, just not so easily discoverable given the layers of gotchas (i.e. types vs types_or and the inability to combine files and types_or via OR).

jalaziz avatar May 30 '24 01:05 jalaziz

pre-commit's design of the filters does not intend to support target files plus the config files for a hook -- it's really meant to just be the target files (and the individual who is updating the config files should be running pre-commit run --all-files themselves separately when updating! (or rely on some CI process)). including both there really doesn't work because the tool will receive the config files as positional arguments

as such I don't really see a need for this particular special identification in identify and so for now I'm going to decline this

asottile avatar Jul 07 '24 23:07 asottile

I understand your reasoning, but I think the buf pre-commit hooks is a good example. While I understand the filtering is meant to actually filter the files that are passed to the hook, it's not uncommon to use filtering as a method of detection (e.g. "only run this hook if these files have changed). In fact, all buf hooks have pass_filenames: false and use types purely as a trigger.

I do agree that's not reason enough to reconsider this.

What I do think is a reason, though, is the user experience of wanting to use a third-party official hook (e.g. buf), wanting to extend it with files and then finding out it's all broken because types and files mix in a somewhat unexpected way (until you've read the docs). After reading the docs, you then find yourself needing to lookup the source of the third-party hook, coming to this repo to determine what the correct regex is for the types used by the third-party hook, then recreate all of it in files directive. On top of all that, due to the extra matching that types can do, it may actually be impossible to recreate types as `files.

Anyway, I do understand your opinion here, but I hope you see my (the user) side of this and that the user experience around files and types with regards to extending third-party hooks is a broken one.

jalaziz avatar Jul 08 '24 02:07 jalaziz

In fact, all buf hooks have pass_filenames: false and use types purely as a trigger.

then the buf hooks are implemented incorrectly and miss most of the point of the framework -- perhaps this is why you're having such a hard time with them.

Anyway, I do understand your opinion here, but I hope you see my (the user) side of this and that the user experience around files and types with regards to extending third-party hooks is a broken one.

it's exceedingly rare to do so, and even then it is documented with examples.

again it should feel wrong because you are probably holding it wrong if you're reaching for such a thing

asottile avatar Jul 08 '24 03:07 asottile