Skip to content

Conversation

vihar
Copy link
Contributor

@vihar vihar commented Sep 10, 2025

Summary

  • add formatURLForDisplay helper to strip protocol and trailing slash from URLs

Testing

  • pnpm --filter=@plane/utils check:lint
  • pnpm --filter=@plane/utils check:types (fails: Cannot find module '@plane/types')
  • pnpm --filter=@plane/utils check:format
  • pnpm --filter=@plane/utils test (no tests found)

https://chatgpt.com/codex/tasks/task_e_68c1e070b158832a8f20c8484496c613

Summary by CodeRabbit

  • New Features
    • Introduced a utility to format URLs for display, producing cleaner, readable hosts and gracefully handling invalid inputs.
  • Documentation
    • Expanded guidance and examples for top-level domain extraction to clarify behavior and usage.

@Copilot Copilot AI review requested due to automatic review settings September 10, 2025 20:38
Copy link
Contributor

coderabbitai bot commented Sep 10, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Introduces a new exported helper formatURLForDisplay in packages/utils/src/url.ts to present URLs without protocol, using URL.host when valid and extractHostname as fallback; returns empty string for falsy input. Also expands JSDoc for extractTLD; no functional changes to other utilities.

Changes

Cohort / File(s) Summary
URL utilities update
packages/utils/src/url.ts
Added formatURLForDisplay(url: string): string with protocol stripping and fallback to extractHostname; returns empty string for falsy inputs. Enhanced JSDoc for extractTLD; no logic changes to existing functions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant URLUtils as url.ts

  Caller->>URLUtils: formatURLForDisplay(url)
  alt url is falsy
    URLUtils-->>Caller: ""
  else url is valid URL
    URLUtils->>URL: new URL(url)
    URLUtils-->>Caller: parsed.host
  else url invalid (throws)
    URLUtils->>URLUtils: extractHostname(url)
    URLUtils-->>Caller: hostname
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • prateekshourya29
  • sriramveeraghanta

Poem

A rabbit nibbles code by night,
Trims the schemes, keeps hosts in sight.
If URLs wobble, fear no plight—
Hostnames hop in, tidy and bright.
Docs now clearer; all feels right.
Thump-thump! Another concise byte. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description deviates substantially from the repository’s required template by using its own headings and omitting the mandatory “Type of Change” checkbox section, “Description” heading, and other specified sections such as “Screenshots and Media” and “References.” Because the structure and content of the description do not align with the provided template, it is missing critical information and cannot be considered complete. Please revise the description to match the repository template exactly by including the “Description” section with detailed change information, the “Type of Change” checklist with appropriate boxes checked, and the “Screenshots and Media,” “Test Scenarios,” and “References” headings, ensuring each required section is fully populated.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title Check ✅ Passed The title concisely summarizes the primary change by indicating the addition of a URL display formatter within the utils package, follows conventional commit syntax, and includes the relevant ticket reference. It clearly reflects the key implementation without extraneous detail or vague terminology. This makes it easy for reviewers and future readers to understand the intent of the pull request at a glance.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/add-formaturlfordisplay-function

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new utility function formatURLForDisplay that creates a clean, user-friendly representation of URLs by stripping the protocol and trailing slashes.

  • Adds formatURLForDisplay function that returns the host portion of valid URLs or falls back to hostname extraction for invalid URLs
  • Uses the existing extractHostname helper as a fallback when URL parsing fails
  • Includes comprehensive JSDoc documentation explaining the function's behavior and purpose

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +101 to +103
* Returns a readable representation of a URL by stripping the protocol
* and any trailing slash. For valid URLs, only the host is returned.
* Invalid URLs are sanitized by removing the protocol and trailing slash.
Copy link
Preview

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The documentation mentions 'stripping trailing slash' but the implementation doesn't actually handle trailing slashes. The new URL(url).host property and extractHostname function don't remove trailing slashes from the host portion. Consider updating the documentation to accurately reflect what the function does, or implement trailing slash removal if that's the intended behavior.

Copilot uses AI. Check for mistakes.

* Invalid URLs are sanitized by removing the protocol and trailing slash.
*
* @param url - The URL string to format
* @returns The formatted domain for display
Copy link
Preview

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The return description says 'formatted domain' but the function returns the host (which includes port numbers if present). Consider changing this to 'The formatted host for display' to be more accurate, since new URL(url).host includes both hostname and port.

Suggested change
* @returns The formatted domain for display
* @returns The formatted host for display

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/utils/src/url.ts (1)

100-116: Confirm intent: function currently drops path/query; PR description says only strip protocol and trailing slash

If the goal is to keep the path (minus a trailing slash) and just remove the scheme, this implementation is too aggressive (it returns only the host). Suggest updating the function to preserve pathname for display, trim input, and safely handle non-URL strings (including credentials) in the fallback.

-export function formatURLForDisplay(url: string): string {
-  if (!url) return "";
-
-  try {
-    return new URL(url).host;
-  } catch (_error) {
-    return extractHostname(url);
-  }
-}
+export function formatURLForDisplay(url: string): string {
+  if (!url) return "";
+  const input = url.trim();
+  if (!input) return "";
+
+  try {
+    const u = new URL(input);
+    // Keep email address for mailto: links; otherwise show host plus pathname (no trailing slash).
+    if (u.protocol === MAILTO_PROTOCOL) return u.pathname;
+    const path = u.pathname === "/" ? "" : u.pathname.replace(/\/+$/, "");
+    return u.host + path;
+  } catch {
+    // Fallback for strings without a valid URL object: remove protocol, strip credentials, and trailing slash.
+    let s = input.replace(PROTOCOL_REGEX, "");
+    const at = s.indexOf("@");
+    if (at !== -1) s = s.substring(at + 1);
+    return s.replace(/\/+$/, "");
+  }
+}

Quick test cases to confirm expected behavior:

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0f7bfdd and 4283f43.

📒 Files selected for processing (1)
  • packages/utils/src/url.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build and lint web apps
  • GitHub Check: Analyze (javascript)

@vihar vihar changed the title feat(utils): add URL display formatter [WEB-4881] feat(utils): add URL display formatter Sep 10, 2025
Copy link

makeplane bot commented Sep 10, 2025

Linked to Plane Work Item(s)

This comment was auto-generated by Plane

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

Successfully merging this pull request may close these issues.

3 participants