https://pantsbuild.org/ logo
Join Slack
Powered by
so maybe still print the parameters of the rule, b...
# development
h
so maybe still print the parameters of the rule, but only the types of those parameters
đź‘Ť 1
w
almost. it additionally suggests that some types are worth rendering
ie, maybe those that implement a marker trait.
🔥 1
đź‘– 1
and if not a marker trait, then a literal python baseclass with a “what is my end user string representation”… and if doesn’t have one, just render the type.
h
I'm worried about adding that API adding an additional source of complexity for rule-writers and also people who hack on the engine
pants already has several ways of representing an object as text to show in a rule
w
no, only one… EDIT: two.
python’s 
repr
or 
str
these are python objects, not rust objects.
a 
Param
is always a python object.
h
right, the fundamental problem here is that the default 
str
of some of the python objects that can be `Param`s is gigantic, which (at least in this case) is coming from the new target API
w
@hundreds-breakfast-49010: right, but it is also the case that some are never useful to render. that’s the real reason i suggest the marker interface/class/trait
the OptionsBootstrapper, for example.
you don’t want to change its repr/str, but you do want to skip it when showing Params to users…. which, BTW: is this method: https://github.com/pantsbuild/pants/blob/de7d5c38363fd0b44be4b277e732a2975fa461a9/src/rust/engine/src/core.rs#L212-L216
a
oh yeah, so do you mean literally like 
def end_user_string(self):
or something
h
in this specific error message I don't think any of the types are useful, and I'm inclined to agree with eric that even the exact rule isn't very useful
probably the most useufl thing for the user would be a list of what specific files changed, but that would require a lot of 
graph
-level replumbing to make that information available
which I think isn't a pre-2.0 priority
a
i agree with eric that the exact rule isn't very useful
w
@hundreds-breakfast-49010: ok, but the reason that this is linked to the other issue is that we still don’t have a way for a rule to have a useful string itself.
h
@aloof-angle-91616 we already have a function like that for `@rule`s, that's part of what I was getting at when I was saying that pants has several ways to represent python objects that are hard to keep straight
w
@hundreds-breakfast-49010: right. but when you say that the filesystem changes should use “some different strategy”, you’re suggesting adding a new way to display them (i think?)… i’m suggesting making the existing way to display them more useful.
a
ok but do we have anything that can be used to construct hygeinic error messages? i was under the impression there was a vague consensus (? confirm?) that error messages currently contain a lot of private information that shouldn't be exposed to end users
đź‘Ť 1
w
@aloof-angle-91616: that’s what is described in this thread. a marker trait/interface.
h
your suggestion if I understand it correctly is to create a new python level marker class that some types that can be input parameters to a 
@rule
will subclasss. and then we would add functionality to only print a string representation of an individual 
Param
if the python value is of that subclass
w
correct.
h
and my thinking is that that's a niche case for a python-level marker class. how is a rule-author supposed to know whether a given parameter to their rule should or should not implement that marker class? it's hard to think about what pants engine debug or error message the rule might eventually wind up in
đź‘Ť 1
w
@hundreds-breakfast-49010: easily, i think.
OptionsBootstrapper? no. Address? yes. AddressSpecs? yes. etc
although perhaps the include/exclude by default could be swapped if there are enough “yeses”.
h
in this case, someone writing the 
black_lint
rule would probably implement this marker for 
BlackRequest
, thnking that's the only interesting parameter to that rule
but it's exactly 
BlackRequest
that is gigantic, which is causing so much text to be dumped
a
@hundreds-breakfast-49010 what i would love is if we could somehow lean on the existing 
@rule
formatting API and then access the dynamic context of the current rule to know how to format individual params, if there were any overrides
h
right, and I think we do eventually want something like this. but not until after v2
a
that might require every rule writer to write these overloads for each rule that may throw an error inside of it unless we do something really complicated
if we attach the formatting method to param type definitions instead, we can just add a single method and then expect to be able to write rules without having to think about whether each individual rule deserves a formatting method
that's a strawman
because i just made up a bad implementation lol
w
right… putting it on the type avoids making it a per-rule decision.
ok, disappearing for real now.
a
will think more about this. i think that it's very idiomatic to make something like a python base class with a special formatting method, and that it could help make error messages better. being able to write more maintainable and more productive error messages sounds like something that could improve the quality in the run up to the 2.0 release.
Open in Slack
PreviousNext
Powered by