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

yoink qtcreator's ansi handler #105

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

fundies
Copy link
Contributor

@fundies fundies commented Mar 15, 2020

No description provided.

if (startPos < text.count())
outputTextBrowser->textCursor().insertText(text.mid(startPos), format);
});

Copy link
Contributor

@RobertBColton RobertBColton Mar 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you hooked this to output text browser's insert signal or w/e then ANSI escapes would be colored for all plugins instead of just the server plugin. Also we may want an option to disable ANSI later and I'm not sure how granular such an option should be, I think Visual Studio has one to disable its color output.

QList<Utils::FormattedText> txts = ansiHandler.parseText(QString::fromStdString(log.message() + "\n"));
for (auto& str : txts) {
emit LogOutput(str.text, str.format);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ANSI escape stuff itself could have been made a plugin and more loosely coupled but I suppose that might be overkill.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I guess this is fine because now all plugins have ability to log formatted output of their own.


static QColor ansiColor(uint code)
{
if (code < 8) return QColor();
Copy link
Contributor

@RobertBColton RobertBColton Mar 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use R_EXPECT here, albeit this tightly couples this component with the rest of our code base making it harder to pull updates to this source from Qt again.

#define R_EXPECT(cond, ret) \

Rather than change this source, what we could do actually is wrap QTC_ASSERT to R_EXPECT so we can easily pull updated versions of this source in the future.

// strippedText always starts with "\e["
QString strippedText = input.text.mid(escapePos);
while (!strippedText.isEmpty()) {
while (strippedText.startsWith(escape)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i just used older version


// \e[K is not supported. Just strip it.
if (strippedText.startsWith(eraseToEol)) {
strippedText.remove(0, 1);
Copy link
Contributor

@RobertBColton RobertBColton Mar 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you comment out just this single line (not the continue!) you will see the letter k on both sides of text that you would expect to be colored. That means GCC is coloring its output using specific ANSI escapes which this ANSI escape handler does not support. That is why color output is still not working while it is correctly bold and such.

@RobertBColton
Copy link
Contributor

RobertBColton commented Mar 15, 2020

Ok after some testing, I don't have any idea how Qt Creator manages to keep the view at the bottom without applying the text cursor like I do above. It seems like they do somewhere for the "Compile Output" window in Qt Creator but I can't find where.

However, I dislike doing it this way because this is how LGM did it, and it doesn't let you scroll up at all while output is coming (very annoying!). I like the behavior of QTextEdit::append and QPlainTextEdit::append where if you scroll up, the viewport stays there, but the text cursor continues moving down and appending new text.
https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qtextedit.cpp?h=dev#n2735
https://code.qt.io/cgit/qt/qtbase.git/tree/src/widgets/widgets/qplaintextedit.cpp?h=dev#n3047

This is much closer to how a terminal actually behaves and in fact is exactly what my testing shows in MSYS2 for me.

@JoshDreamland
Copy link
Member

If fundies insists, Robert, let him use this. I think it's ugly as stop-gaps go, but it'll work.

Another alternative is this library: https://github.com/theZiz/aha/blob/master/aha.c

Seems we could copy-paste that instead. It's MPL-licensed, which is closer to LGPL. It might support that weird \e[k thing. And it feels like it's an easier library to contribute back to if it has shortcomings.

@RobertBColton
Copy link
Contributor

That weird \e[K thing is exactly the reason the color doesn't actually work, only bold does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants