error messages - do not repeat class names twice in our error messages #20165
Conversation
demerphq
force-pushed
the
yves/no_repeat_package
branch
2 times, most recently
from
455b620 to
e693667
Compare
|
I really think this should be merged. The cognitive load from the double classname when the classname is long is quite high and I dont think repeating it twice really adds much value. |
|
What are peoples' thoughts on this? If we want it and demerphq is busy, I'm happy to rebase it. |
|
When considering this, please consider cases where the class name is very
long, such as in generated class names or whatnot. Some of what we do seems
reasonable when the class name is short, but ends up being
counterproductive when it is long.
Yves
…On Thu, 5 Dec 2024 at 14:38, Richard Leach ***@***.***> wrote:
What are peoples' thoughts on this? If we want it and demerphq is busy,
I'm happy to rebase it.
—
Reply to this email directly, view it on GitHub
<#20165 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAZ5R22XYEPIDYWU67JGXD2EBJORAVCNFSM6AAAAABTCPT5SWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRQGM2TEMBWGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
|
I generally agree with the principle that each message only needs to name the class once. This PR is a small simple change that implements the idea well. I say go for it. |
Showing the classname twice in the error message seems friendly when the
class name is short, but when it is long enough that the line wraps the
duplication just increases cognitive load understand the error message.
This is especially the case when the value is a total error and contains
gibberish or a long binary string or such things, something that is all
to common with this type of error. Even with the recently merged eliding
of names the duplication means that an error message can be 2k long in
the worst case, mostly because of the unnecessary duplication.
As a compromise we can simply replace the second invocation of the class
name by saying "it" instead, and that is what this patch does. The error
message is still friendly, but not repetitive. We could also use "the
package" if people preferred.
Thus:
$ perl -le'("x" x 50)->new()'
Can't locate object method "new" via package "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
(perhaps you forgot to load "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"?) at -e line 1.
Turns into:
$ ./perl -le'("x" x 50)->new()'
Can't locate object method "new" via package "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
(perhaps you forgot to load it?) at -e line 1.
khwilliamson
force-pushed
the
yves/no_repeat_package
branch
from
e693667 to
3cf01b3
Compare
|
Can we also substr the class name in cases where it contains control characters or is longer than a "reasonable" class name ("Reasonable" could come from the longest path the OS allows, for example) It would be nice in cases where you call a method on a string that's not a class name, especially in webby cases, like jpgs or for large json blobs: my $jph_binary = slurp glob "~/pics/kitten.jpg";
$jpg_binary->anything(); |
|
@guest20 That's not a bad idea, but I wouldn't do it in this PR. Can you open a new bug? |
That's literally what QUOTEDPREFIX is for. |
Indeed. |
Showing the classname twice in the error message just increases
cognitive load understanding the message when the class/package name
is more than a few components long, and can easily make what should be
a simple one line error message wrap and be unreadable.
We can simply replace the second invocation of the class name by saying
"it" instead, and that is what this patch does. It is still friendly,
but not repetitive.
Thus:
$ perl -le'("x" x 50)->new()'
Can't locate object method "new" via package "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
(perhaps you forgot to load "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"?) at -e line 1.
Turns into:
$ ./perl -le'("x" x 50)->new()'
Can't locate object method "new" via package "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
(perhaps you forgot to load it?) at -e line 1.