Skip to content

Conversation

@maxandersen
Copy link

@maxandersen maxandersen commented Dec 19, 2025

DO NOT MERGE - just here to be able to get feedback on wether this right or wrong fixes in core ;)

public static final int COLORS_RGB = 2;
public static final int COLORS_INDEXED = 5;

// OSC 8 hyperlinks (BEL-terminated)
Copy link
Author

Choose a reason for hiding this comment

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

links are useful and osc-8 are commonly supported so i assume makes sense to add here?


public static String foregroundIndexed(int index) {
return FOREGROUND_COLORS + ":" + COLORS_INDEXED + ":" + index;
// Standard SGR: 38;5;<n>
Copy link
Author

Choose a reason for hiding this comment

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

not sure who is right here - it does not seem to matter but online i find ; being used not : - which one is correct/expected?

}
if ((currentStyleState & MASK_FG_COLOR) != (state & MASK_FG_COLOR)) {
styles.add(fgColor().toAnsiFg());
styles.add(extractSgrParams(fgColor().toAnsiFg()));
Copy link
Author

Choose a reason for hiding this comment

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

this extact call is super inefficient - but afacs its done to avoid getting style start ansi code added constantly so it strips it and leave just the style code?

@@ -0,0 +1,660 @@
package org.codejive.twinkle.core.text;
Copy link
Author

Choose a reason for hiding this comment

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

this class needs major overhaul - only useful part is ability to run and see the output which currently looks correct - but its doing dumb things.

return EMOJI.get(name);
}

private static final Map<String, String> EMOJI = new HashMap<>();
Copy link
Author

Choose a reason for hiding this comment

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

TODO: having a 1000+ map.put in a static class init probably aint good for performance.

int x = 0;
for (Span span : spans) {
span.render(canvas.view(x, 0, span.length(), 1));
x += span.length();
Copy link
Author

Choose a reason for hiding this comment

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

haven't yet verified this one but seems relevant to actually push x forward when rendering?/

return length;
}

public String text() {
Copy link
Author

Choose a reason for hiding this comment

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

this shouldn't be needed. caused by dumb code in Console.java so ignore for now

Style style = Style.of(styleBuffer[i]);
style.toAnsiString(sb);
// Transition from the previous style to the new one (prevents style "bleed" and
// keeps output compact, closer to JLine).
Copy link
Author

Choose a reason for hiding this comment

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

this I think make sense but curious if you had reasons not to do this?

+ Ansi.style(Ansi.ITALICIZED)
+ "abcde"
+ Ansi.style(Ansi.UNDERLINED)
+ Ansi.style(Ansi.NOTITALICIZED, Ansi.UNDERLINED)
Copy link
Author

Choose a reason for hiding this comment

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

not sure if this is right - but needed to pass now that the rendering closes spans

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.

1 participant