Repository navigation
Fix --exclude-imports with --path or --type - #4775
oliversun9 wants to merge 17 commits into
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
54d0b64 to
0057121
Compare
emcfarlane
left a comment
emcfarlane
left a comment
There was a problem hiding this comment.
lgtm, small comments on wording.
| // 3. A file that is not itself a target, but is (possibly transitively) | ||
| // imported by a target file. |
There was a problem hiding this comment.
Maybe could clarify this wording a bit. The not itself a target reads to me as a file is not an import if its not imported which is circular. Maybe?
3. A file that a target file depends on.
| // When a type filter is allowed to select types from imported files, as the | ||
| // BSR does, a file that defines a selected type is a target even if it falls | ||
| // under one of the cases above. |
There was a problem hiding this comment.
This too. I think the wording needs to be closer to the FileDescriptor. Something along the lines of a "target file is always set to is_import false, regardless of which Module it derived from".
| // Normally, if `--path a.proto` is passed it's an error to pass `--type B`, because | ||
| // they don't intersect. However, if `--path a.proto` and `--type pkg` is passed and | ||
| // both a.proto and b.proto are in the pkg package, then it's valid. This time `--type pkg` | ||
| // does not make b.proto a target because it's excluded by `--path a.proto` already. |
There was a problem hiding this comment.
Maybe describing as the union of paths intersected with the union of types would help?
|
|
||
| - Fix LSP hover and highlighting of CEL expressions whose proto strings contain escape sequences, | ||
| which placed the ranges of string literals, and of tokens after a `\u` or `\U` escape, wrongly. | ||
| - Fix a bug where a file imported by the file of a type specified by `--type`, without defining a |
There was a problem hiding this comment.
Nit: could maybe wordsmith this a bit. The a bug is redundant. I think we want to just comment something like:
- Fix `buf build --type=X` filters to correctly mark non-targeted files as imports.
But should expand to cover path interactions and image changes.
The correct order of filtering: