Skip to content

Upgrade glamour to fix hyperlinks (OSC8) - #281

Open
heaths wants to merge 1 commit into
cli:trunkfrom
heaths:issue188
Open

Upgrade glamour to fix hyperlinks (OSC8)#281
heaths wants to merge 1 commit into
cli:trunkfrom
heaths:issue188

Conversation

@heaths

@heaths heaths commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #188

@heaths

heaths commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

This is stacked atop #280 so this either needs to be split into a separate PR atop trunk or turned into a stack / that PR merged first. I'm building up my own replacement module with fixes I need for extensions we use in workflows and custom agent instructions.

@heaths
heaths marked this pull request as ready for review August 14, 2026 20:59
@heaths
heaths requested a review from a team as a code owner August 14, 2026 20:59
@heaths
heaths requested review from BagToad and removed request for a team August 14, 2026 20:59
@heaths

heaths commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

This is part of a stack but not showing here because stacks aren't enabled for this repo it seems:

Syncing PRs ...
✓ PR #280 (issue279) — Open
✓ PR #281 (issue188) — Open
⚠ Stacked PRs are not enabled for this repository

✓ Branches synced
  Stacked on trunk (450618a)

I can split these later if needs be, but stacks at least make it easier to rebase --onto (automatic vs. manual).

@BagToad

BagToad commented Aug 14, 2026

Copy link
Copy Markdown
Member

I would prefer this dependency bump on its own and not stacked with the unrelated change please. If you have the time; proof that it doesn't break tests in cli/cli would also be an asset to decrease the review burden. If it does break tests, some investigation into why is in order as well.

I don't understand the "replacement module" comment, sorry. If that's something you meant for us to action, can you expand on that?

@heaths

heaths commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

I would prefer this dependency bump on its own and not stacked with the unrelated change please.

@BagToad if you're open to taking it, I can unstack them. I mainly did that so I could simply build against the head of the stack against my fork. Less for the stack and more for the natural HEAD so I can build against all changes.

I'm making a similar change to the cli right now for cli/cli#3718. There were a few expected \e[0m -> \e[m conversions (effectively equivalent) but also a couple other changes with the newer version of lipgloss that you already used in some places (your go.mod for gh uses both the v2 and v0 versions of lipgloss.

But how would this change impact the cli directly? I see you are using x/markdown. I should probably split this PR out and use a replacement module (literally "replace" in the go.mod e.g., https://github.com/heaths/gh-minimize/blob/710a1ab366b560af52e84846379a81df05ed00b5/go.mod#L14) there as well.

@heaths

heaths commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

The lint issue is that golangci-lint timed out or otherwise couldn't connect to the schema. Odd thing for it to fail on; however, I can't rerun the failed check myself. I imagine it'd pass on a subsequent run. The file exists—I just checked in.

@heaths

heaths commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Since I can't rerun the failed test I simply amended the commit and did a force push.

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.

Pick up new version of Glamour to avoid breaking hyperlinks

2 participants