refactor: make show implement one of the methods#3402
refactor: make show implement one of the methods#3402peter-jerry-ye wants to merge 5 commits intomainfrom
Conversation
|
Note : it seems that this requires compiler fix for the display of errors |
| output(Self, &Logger) -> Unit | ||
| // By default `output` is implemented using `to_string`, | ||
| // but some types, such as `String` and `Char`, may override `output`, | ||
| // for different (escaped) behavior when composed into larger structures. |
There was a problem hiding this comment.
this is no longer the case? we have two purely for efficiency?
There was a problem hiding this comment.
It depends on how we are going to phase out the transition.
Coverage Report for CI Build 3723Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.03%) to 94.994%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions19 previously-covered lines in 5 files lost coverage.
Coverage Stats
💛 - Coveralls |
| fn Error::to_string(self : Error) -> String { | ||
| @debug.to_string(self) | ||
| } |
There was a problem hiding this comment.
We are using debug.to_string to display. But this can cause major breakage for errors that have display, such as the errors in the argparse. Users might find that the program no longer prints the errors well. Though no one should depend on the error message's content.
4 participants
We are going to change the behavior of
Showso that it is purely for displaying. So there will not be special handling forStringorCharany more. As a result, theoutputandto_stringwould behave the same. Thus people only need to implement one of the methods.This PR also migrates some of the builtin types' implementations to
to_string, because useoutputwould forcefully upcastStringBuiderto&Logger, which will increase code in some cases.