Skip to content

fix(renderer): join multiline external links#2503

Merged
arnestrickmann merged 2 commits into
mainfrom
eng-1576-multiline-links
Jun 12, 2026
Merged

fix(renderer): join multiline external links#2503
arnestrickmann merged 2 commits into
mainfrom
eng-1576-multiline-links

Conversation

@janburzinski

Copy link
Copy Markdown
Collaborator

Description

  • fixes multiline external links

Screenshot/Recording (if applicable)

https://streamable.com/l1mccd

Checklist
  • I kept this PR small and focused
  • I ran a self-review before opening this PR
  • I ran the relevant local checks or explained why not
  • I updated docs when behavior or setup changed
  • I added or updated tests when behavior changed, or explained why not
  • I only added comments where the logic is not obvious
  • I used Conventional Commits for commit
    messages and, when possible, the PR title

@greptile-apps

greptile-apps Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds multiline URL joining to normalizeExternalHttpUrl via a new joinMultilineUrl helper that inspects each line break and decides whether to remove it based on context cues (breaksUrlSegment, hasIndent, startsIndentedLabel). A new trimTrailingText helper replaces the previous single-line whitespace slice.

  • joinMultilineUrl correctly handles the Linear UUID-path case and the previously-flagged indented-label false-join is now blocked by the startsIndentedLabel guard.
  • The startsIndentedLabel guard is only reached when !breaksUrlSegment; when the URL breaks at a separator character (hyphen, dot, ?, etc.) the guard is bypassed, silently corrupting URLs like https://example.com/project- Status: Active.
  • Tests cover the happy path and the alphanumeric-predecessor label case, but not the separator-predecessor label case.

Confidence Score: 4/5

The core joining logic works for the primary Linear UUID-path use case, but one code path in joinMultilineUrl can silently corrupt URLs that break at a separator character followed by an indented label.

The breaksUrlSegment branch joins unconditionally without consulting startsIndentedLabel, so a URL like https://example.com/name-
Status: Active is collapsed to https://example.com/name-Status with no error or warning — a real wrong-URL scenario on input shapes common in Linear attachment views.

apps/emdash-desktop/src/renderer/lib/external-url.ts — the joinMultilineUrl decision tree and the test file both need a case covering a separator-terminated URL followed by an indented label.

Important Files Changed

Filename Overview
apps/emdash-desktop/src/renderer/lib/external-url.ts Adds joinMultilineUrl and trimTrailingText helpers; the startsIndentedLabel guard correctly blocks indented labels after alphanumeric URL endings but is bypassed when the previous character is a URL separator (breaksUrlSegment path), leaving a false-join case unhandled.
apps/emdash-desktop/src/renderer/lib/external-url.test.ts Adds two new test cases covering multiline URL joining and indented label detection, but no test covers a URL ending in a separator character followed by an indented label.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[joinMultilineUrl - match linebreak] --> B{previousChar or nextChar missing?}
    B -->|Yes| C[Keep linebreak]
    B -->|No| D{nextChar matches URL_CONTINUATION?}
    D -->|No| C
    D -->|Yes| E{breaksUrlSegment - prev is separator}
    E -->|Yes| F[Join unconditionally - startsIndentedLabel NOT checked]
    E -->|No| G{hasIndent - lineBreak ends with space or tab}
    G -->|No| C
    G -->|Yes| H{startsIndentedLabel matches Word-colon pattern}
    H -->|Yes| C
    H -->|No| I[Join]
Loading

Reviews (2): Last reviewed commit: "fix(renderer): avoid joining indented UR..." | Re-trigger Greptile

Comment thread apps/emdash-desktop/src/renderer/lib/external-url.ts
Comment thread apps/emdash-desktop/src/renderer/lib/external-url.test.ts
@janburzinski

Copy link
Copy Markdown
Collaborator Author

@greptileai

@arnestrickmann arnestrickmann left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thanks for the fix

@arnestrickmann arnestrickmann merged commit d940933 into main Jun 12, 2026
1 check passed
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.

2 participants