Fix #152: Fix pull request links in bot answers including trailing punctuation #153

Open

typosaurus wants to merge 1 commits from typosaurus/ticket-152 into master

typosaurus commented

2026-07-28 22:03:02 +02:00

Collaborator

Resolves #152.

Plan

Implementation Plan

Files to modify and changes required:

1. `devplacepy/rendering.py` – lines 141–162 (`_embed_url`)

After retrieving the matched URL, strip trailing punctuation characters from the end:
- Use regex [.,;:!?)\]}\'"]+$ to identify any trailing punctuation.
- If a match is found, separate the cleaned URL and the trailing punctuation string.
In the returned HTML, place the cleaned URL inside the <a> tag’s href and visible text, then append the trailing punctuation as plain text after the closing </a> tag.
Also update _TOKEN_RE (line 66–69) to avoid changing the regex itself – the fix is applied during rendering, not during token matching. (The current pattern is correct for token extraction; the stripping step is handled downstream.)

Pseudocode for _embed_url:

def _embed_url(url, display=None):
    # Strip trailing punctuation
    trail = ""
    match = re.search(r'[.,;:!?)\]}\'"]+$', url)
    if match:
        trail = match.group()
        url = url[:match.start()]
    escaped = html.escape(url)
    text = html.escape(display or url)
    return f'<a href="{escaped}" ...>{text}</a>{trail}'

2. `devplacepy/static/js/ContentRenderer.js` – lines 161 and `processMedia()` / `urlToEmbed()` (line 243)

In the same function that creates <a> tags from bare URLs, apply an identical stripping step before building the anchor.
Add a helper function (e.g., _stripTrailingPunct(url)) that returns {cleaned, trail} using the same regex /[.,;:!?)\]}\'"]+$/.
When constructing the anchor, place the cleaned URL in href and text content, then append trail as a text node (or string concatenation outside the <a> element).

Example in urlToEmbed() (line 243):

function urlToEmbed(matchedUrl) {
  const { cleaned, trail } = stripTrailingPunct(matchedUrl);
  return `<a href="${cleanUrl(cleaned)}" target="_blank">${escapeHtml(cleaned)}</a>${trail}`;
}

3. Add unit tests (at minimum for the server‑side change)

File: tests/unit/rendering.py
- Add a test case that provides a string like "See https://github.com/user/repo/pull/123." and verifies the rendered HTML contains <a href="https://github.com/user/repo/pull/123">...pull/123</a>. (period outside).
- Also test other trailing punctuation: ,, !, ?, ).
For the client‑side, if a test harness exists (none cited), add a similar test to the existing suite or note that manual verification is acceptable given the proven pattern.

4. Remove or update prior unrelated changes (if any were left from earlier attempts)

Ensure no leftover schema or project_link code from attempts #150 persists. The fix is purely in rendering logic.

Definition of Done

Both _embed_url and the client‑side URL renderer strip trailing punctuation from bare URLs, and the punctuation is placed outside the <a> element.
A unit test exists in tests/unit/rendering.py that validates the server‑side fix for at least ., ,, !, ?, ).
The project’s test command passes with zero failures when run as:
```
pip install -e '.[dev]' -q && python -m pytest tests/unit/ tests/api/devrant/rants.py
```
(Replace {test_files} with the actual paths; the full suite should pass, but at a minimum the rendering unit tests and the previously‑failing test_rant_serialization_includes_project must succeed.)
No regressions are introduced in the rendering of bare URLs without trailing punctuation, markdown links, or any other existing functionality.
Manual smoke test: In a running instance, post a comment ending with a bare URL and a period – the rendered link must point to the correct URL and the period must appear after the link, not inside it.

Opened automatically by Typosaurus.

Resolves #152. ## Plan ## Implementation Plan **Files to modify and changes required:** ### 1. `devplacepy/rendering.py` – lines 141–162 (`_embed_url`) - After retrieving the matched URL, strip trailing punctuation characters from the end: - Use regex `[.,;:!?)\]}\'"]+$` to identify any trailing punctuation. - If a match is found, separate the cleaned URL and the trailing punctuation string. - In the returned HTML, place the cleaned URL inside the `<a>` tag’s `href` and visible text, then append the trailing punctuation as plain text after the closing `</a>` tag. - Also update `_TOKEN_RE` (line 66–69) to avoid changing the regex itself – the fix is applied during rendering, not during token matching. (The current pattern is correct for token extraction; the stripping step is handled downstream.) **Pseudocode for `_embed_url`:** ```python def _embed_url(url, display=None): # Strip trailing punctuation trail = "" match = re.search(r'[.,;:!?)\]}\'"]+$', url) if match: trail = match.group() url = url[:match.start()] escaped = html.escape(url) text = html.escape(display or url) return f'<a href="{escaped}" ...>{text}</a>{trail}' ``` ### 2. `devplacepy/static/js/ContentRenderer.js` – lines 161 and `processMedia()` / `urlToEmbed()` (line 243) - In the same function that creates `<a>` tags from bare URLs, apply an identical stripping step before building the anchor. - Add a helper function (e.g., `_stripTrailingPunct(url)`) that returns `{cleaned, trail}` using the same regex `/[.,;:!?)\]}\'"]+$/`. - When constructing the anchor, place the cleaned URL in `href` and text content, then append `trail` as a text node (or string concatenation outside the `<a>` element). **Example in `urlToEmbed()` (line 243):** ```javascript function urlToEmbed(matchedUrl) { const { cleaned, trail } = stripTrailingPunct(matchedUrl); return `<a href="${cleanUrl(cleaned)}" target="_blank">${escapeHtml(cleaned)}</a>${trail}`; } ``` ### 3. Add unit tests (at minimum for the server‑side change) - **File:** `tests/unit/rendering.py` - Add a test case that provides a string like `"See https://github.com/user/repo/pull/123."` and verifies the rendered HTML contains `<a href="https://github.com/user/repo/pull/123">...pull/123</a>.` (period outside). - Also test other trailing punctuation: `,`, `!`, `?`, `)`. - For the client‑side, if a test harness exists (none cited), add a similar test to the existing suite or note that manual verification is acceptable given the proven pattern. ### 4. Remove or update prior unrelated changes (if any were left from earlier attempts) - Ensure no leftover schema or project_link code from attempts #150 persists. The fix is purely in rendering logic. --- ## Definition of Done - [ ] Both `_embed_url` and the client‑side URL renderer strip trailing punctuation from bare URLs, and the punctuation is placed outside the `<a>` element. - [ ] A unit test exists in `tests/unit/rendering.py` that validates the server‑side fix for at least `.`, `,`, `!`, `?`, `)`. - [ ] The project’s test command passes with zero failures when run as: ```bash pip install -e '.[dev]' -q && python -m pytest tests/unit/ tests/api/devrant/rants.py ``` (Replace `{test_files}` with the actual paths; the full suite should pass, but at a minimum the rendering unit tests and the previously‑failing `test_rant_serialization_includes_project` must succeed.) - [ ] No regressions are introduced in the rendering of bare URLs without trailing punctuation, markdown links, or any other existing functionality. - [ ] Manual smoke test: In a running instance, post a comment ending with a bare URL and a period – the rendered link must point to the correct URL and the period must appear after the link, not inside it. Opened automatically by Typosaurus.

typosaurus added 1 commit 2026-07-28 22:03:03 +02:00