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

[DEFECT] CommonOptions not available when formatting outside a PC #697

Closed
AlexWeinstein92 opened this issue Nov 25, 2024 · 5 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@AlexWeinstein92
Copy link
Collaborator

Symptom

After each run, the plugin saves the Messages that come back from runCommandWithArgs
These messages do not respect the check for commonOptions.noANSIMessages because the options are not passed into the format function or its downstream annotateErrorLine function

Expected Behavior

NoANSIMessages, and all other CommonOptions, should appropriately affect Messages.format even outside of a PlatformContext

Steps To Reproduce

List the steps to reproduce the symptomatic behavior:

  1. Inspect the output Messages of runcommandWithArgs and note that ANSI formatting codes are still present even when noANSIMessages = true
@AlexWeinstein92 AlexWeinstein92 added the bug Something isn't working label Nov 25, 2024
@reid-spencer reid-spencer self-assigned this Nov 25, 2024
Copy link
Contributor

This should have been fixed in this commit:

302091e

Copy link
Contributor

@AlexWeinstein92 - do you concur?

@AlexWeinstein92
Copy link
Collaborator Author

AlexWeinstein92 commented Dec 2, 2024

After further investigation I realize I was off in my diagnosis of the root of the issue I saw.

noANSIMessages is only checked for in annotateErrorLine, which in Message.format is only used to annotate loc for errorLine. However, what is required for the plugin is for noANSIMessages to also affect Message.message - that is to say, the actual String holding the message ought to respect noANSIMessage. I think probably every part of what's returned in format should have a check for noANSIMessages

Copy link
Contributor

There are only two modules that import scala.io.AnsiColor and therefore only two modules that could possibly insert ANSI highlighting. those modules are utils.Logging and language.parsing.RiddlParserInput. Consequently, the Message.format method does not need to have checks for noANSIMessages in it. That method does call RiddlParserInput. annotateErrorLines but its use of ANSI highlighting is guarded by a check on noANSIMessages.

The only use of ANSI highlighting in the Logger occurs in the highlight method which uses an appropriate noANSIMessages highlight to guard the highlighting of the message.

The "actual String holding the message" can only be generated from the Parser or validation routines and they never insert ANSI highlighting because they never import scala.io.AnsiColor.

Consequently, I hold that no ANSI highlighting is being done without heeding the noANSIMessages setting in CommonOptions. If this occurs for you, it must necessarily be the setting of noANSIMessages you've placed in your given statements.

@AlexWeinstein92
Copy link
Collaborator Author

Turns out a refactoring I did is another branch. My current branch uses an old method of producing console output. Apologies for the confusion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants