Skip to content

Extract link URLs to UrlConstants.cs - #49

Open
dasimon wants to merge 2 commits into
OlapPivotTableExtensions:masterfrom
dasimon:upstream/url-constants
Open

dasimon wants to merge 2 commits into
OlapPivotTableExtensions:masterfrom
dasimon:upstream/url-constants

Conversation

@dasimon

@dasimon dasimon commented Apr 22, 2026

Copy link
Copy Markdown

Replaces hardcoded Process.Start URL strings with constants from a dedicated UrlConstants class, making it easy to override URLs in forks without touching MainForm.cs.

Replaces hardcoded Process.Start URL strings with constants from a
dedicated UrlConstants class, making it easy to override URLs in forks
without touching MainForm.cs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 22, 2026 07:51

Copilot AI left a comment

Copy link
Copy Markdown

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 centralizes several UI hyperlink targets by introducing a dedicated UrlConstants class and switching MainForm link handlers to use those constants, making URL changes easier to manage in forks.

Changes:

  • Added UrlConstants with constants for common documentation/project URLs.
  • Updated MainForm link click handlers to call Process.Start(...) with UrlConstants values instead of hardcoded strings.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
OlapPivotTableExtensions/UrlConstants.cs Introduces a central location for URL constants used by the UI.
OlapPivotTableExtensions/MainForm.cs Replaces several hardcoded Process.Start URL strings with references to UrlConstants.
Comments suppressed due to low confidence (1)

OlapPivotTableExtensions/MainForm.cs:1196

  • This block still contains a hardcoded URL in linkUpgradePivotTable_LinkClicked even though the PR description says hardcoded Process.Start URL strings are being replaced with constants. If the intent is to make all outbound URLs overrideable via UrlConstants, consider extracting this URL as well (or clarify the PR description/scope).
        private void linkUpgradePivotTable_LinkClicked(object sender, LinkLabelLinkClickedEventArgs e)
        {
            System.Diagnostics.Process.Start("https://web.archive.org/web/20111202190114/http://office.microsoft.com/en-us/excel-help/working-with-different-pivottable-formats-in-office-excel-HA010167298.aspx");
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1184 to 1186
{
System.Diagnostics.Process.Start("https://olappivottableextensions.github.io");
System.Diagnostics.Process.Start(UrlConstants.ProjectHome);
}

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

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

UrlConstants is referenced here, but both project files in this repo use explicit <Compile Include=...> item lists (non-SDK-style). Unless UrlConstants.cs is added to those lists, the solution will fail to compile (both OlapPivotTableExtensions.csproj and OlapPivotTableExtensions2016.csproj).

Copilot uses AI. Check for mistakes.
Comment on lines +5 to +7
internal const string ProjectHome = "https://olappivottableextensions.github.io";
internal const string CalculationsHelp = "https://olappivottableextensions.github.io/Calculations-Help.html";
internal const string UnsupportedLanguageConfig = "https://olappivottableextensions.github.io/Unsupported-Language-Configuration.html";

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

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

New constants here use PascalCase (e.g., ProjectHome), but existing const fields in this codebase are consistently named in ALL_CAPS_WITH_UNDERSCORES (e.g., Connect.cs:181, MainForm.cs:1065, Impersonater.cs:101). To keep naming consistent, rename these constants to match that convention (and consider renaming UnsupportedLanguageConfig to use the full "Configuration" term to match the URL/handler name).

Suggested change
internal const string ProjectHome = "https://olappivottableextensions.github.io";
internal const string CalculationsHelp = "https://olappivottableextensions.github.io/Calculations-Help.html";
internal const string UnsupportedLanguageConfig = "https://olappivottableextensions.github.io/Unsupported-Language-Configuration.html";
internal const string PROJECT_HOME = "https://olappivottableextensions.github.io";
internal const string CALCULATIONS_HELP = "https://olappivottableextensions.github.io/Calculations-Help.html";
internal const string UNSUPPORTED_LANGUAGE_CONFIGURATION = "https://olappivottableextensions.github.io/Unsupported-Language-Configuration.html";

Copilot uses AI. Check for mistakes.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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