https://pantsbuild.org/ logo
#development
Title
# development
h

hundreds-breakfast-49010

08/19/2020, 8:31 PM
so maybe still print the parameters of the rule, but only the types of those parameters
đź‘Ť 1
w

witty-crayon-22786

08/19/2020, 8:32 PM
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

hundreds-breakfast-49010

08/19/2020, 8:34 PM
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

witty-crayon-22786

08/19/2020, 8:34 PM
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

hundreds-breakfast-49010

08/19/2020, 8:36 PM
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

witty-crayon-22786

08/19/2020, 8:36 PM
@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

aloof-angle-91616

08/19/2020, 8:38 PM
oh yeah, so do you mean literally like
def end_user_string(self):
or something
h

hundreds-breakfast-49010

08/19/2020, 8:38 PM
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

aloof-angle-91616

08/19/2020, 8:39 PM
i agree with eric that the exact rule isn't very useful
w

witty-crayon-22786

08/19/2020, 8:40 PM
@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

hundreds-breakfast-49010

08/19/2020, 8:40 PM
@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

witty-crayon-22786

08/19/2020, 8:41 PM
@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

aloof-angle-91616

08/19/2020, 8:42 PM
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

witty-crayon-22786

08/19/2020, 8:43 PM
@aloof-angle-91616: that’s what is described in this thread. a marker trait/interface.
h

hundreds-breakfast-49010

08/19/2020, 8:43 PM
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

witty-crayon-22786

08/19/2020, 8:43 PM
correct.
h

hundreds-breakfast-49010

08/19/2020, 8:45 PM
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

witty-crayon-22786

08/19/2020, 8:45 PM
@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

hundreds-breakfast-49010

08/19/2020, 8:47 PM
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

aloof-angle-91616

08/19/2020, 8:49 PM
@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

hundreds-breakfast-49010

08/19/2020, 8:50 PM
right, and I think we do eventually want something like this. but not until after v2
a

aloof-angle-91616

08/19/2020, 8:51 PM
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

witty-crayon-22786

08/19/2020, 8:53 PM
right… putting it on the type avoids making it a per-rule decision.
ok, disappearing for real now.
a

aloof-angle-91616

08/19/2020, 8:55 PM
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.