Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings displayed after main exit do not use defined to_text #11569

Closed
radeusgd opened this issue Nov 15, 2024 · 1 comment · Fixed by #11591
Closed

Warnings displayed after main exit do not use defined to_text #11569

radeusgd opened this issue Nov 15, 2024 · 1 comment · Fixed by #11591
Assignees
Labels

Comments

@radeusgd
Copy link
Member

Run the following script:

from Standard.Base import all

type My_Warning
    Value msg
    to_display_text self -> Text = "My_Warning to_display_text: "+self.msg
    to_text self -> Text = "My_Warning to_text: "+self.msg
    pretty self -> Text = "My_Warning pretty: "+self.msg

main =
    one = Warning.attach (My_Warning.Value "ONE") 1
    two = Warning.attach (My_Warning.Value "TWO") 2
    [one, two]

Actual behaviour

It prints

one = 1
  ! Value ONE
two = 2
  ! Value TWO
[1, 2]

Where is the Value ONE coming from? Presumably it is Atom.java::toString.

Expected behaviour

If I override to_text and to_display_text of an atom, I think it's default toString shouldn't show up anywhere.

One of to_text or to_display_text should be used when displaying warnings attached to the values. I guess to_display_text is more appropriate, so it should probably look like this:

one = 1
  ! My_Warning to_display_text: ONE
two = 2
  ! My_Warning to_display_text: TWO
[1, 2]
@JaroslavTulach
Copy link
Member

JaroslavTulach commented Nov 19, 2024

If I override to_text and to_display_text of an atom, I think it's default toString shouldn't show up anywhere.

That's probably correct.

One of to_text or to_display_text should be used when displaying warnings attached to the values.

"One of ... should be used" - looks like how little difference there is between these two methods. You rather override both and hope one of them gets used.

I guess to_display_text is more appropriate

If we only knew what's the intended difference between those two methods!?

The system is trying to call to_text method when computing the warning message.

@JaroslavTulach JaroslavTulach moved this from ❓New to 👁️ Code review in Issues Board Nov 19, 2024
@mergify mergify bot closed this as completed in #11591 Nov 20, 2024
@mergify mergify bot closed this as completed in afe4203 Nov 20, 2024
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Nov 20, 2024
somebody1234 pushed a commit that referenced this issue Nov 21, 2024
…ment` (#11591)

Fixes #11569 by using `.to_text` on the right _warning's value_ (as introduced by #10842) and sharing the code with the instrument.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🟢 Accepted
Development

Successfully merging a pull request may close this issue.

2 participants