support formatter ordering #20
No reviewers
Labels
No Label
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No Milestone
No project
No Assignees
2 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: numtide/treefmt#20
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/formatter-ordering"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Allows specifying the order in which formatters are applied.
Very simple for now, adding a
Before
field to the formatted config which allows the user to say that formatterx
needs to be applied before formattedy
.Signed-off-by: Brian McGee brian@bmcgee.ie
@zimbatm what do you think about allowing the user to configure a formatter as needing to be before another?
Essentially, it restricts things to simple linear behaviour. I can't envision a scenario that would require a more complex config.
In a real-world scenario, you would have
statix
beforedeadnix
for example.I haven't wrapped my head around the concurrent aspect of the code, but the rest looks good.
In some cases, you might want to declare that deadnix is run before both statix and nixpkgs-fmt. The intent can't be expressed with the current design, but this can be added in a comment. I think this is good for the first iteration.
@ -67,0 +72,4 @@
if !ok {
return fmt.Errorf(
"formatter %v is before %v but config for %v was not found",
name, formatter.Before, formatter.Before,
formatter.Before is there 2 times
If you look at the string you can see why it's listed twice
@ -67,0 +74,4 @@
"formatter %v is before %v but config for %v was not found",
name, formatter.Before, formatter.Before,
)
}
You need a second for loop here to check for the dependency cycle.
Yeah I haven't done the dependency cycle check yet
Addressed in
23c86cee02
@ -25,2 +25,4 @@
// Excludes is an optional list of glob patterns used to exclude certain files from this Formatter.
Excludes []string
// Before is the name of another formatter which must process a path after this one
Before string
Personal taste thing: because of how many public/private fields there are, I would split this into a FormatterConfig that only contains the config and serializes/deserializes, and then the Formatter that gets generated from the config. That way, how to interact with the structs in the rest of the code becomes clearer. I don't think the Before field should be mutated in the code for example.
It was a fair point, I've separated things out in
c542655177
In the example you provide,
deadnix
would be beforestatix
which would be beforenixpkgs-fmt
:I agree, this could be more expressive but as a first iteration it would solve some existing problems.
c542655177
tod323715fa8
WIP: support formatter orderingto support formatter ordering