Skip to content

Refactor markdown properties to handle whitespace #559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Eulenhorn wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Refactor markdown properties to handle whitespace #559

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions gamevault/ViewModels/GameViewViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using gamevault.Models;
using gamevault.Models;
using System;
using System.Collections.Generic;
using System.ComponentModel;
Expand Down Expand Up @@ -61,12 +61,27 @@ public bool? IsDownloaded
public string? DescriptionMarkdown
{
get { return descriptionMarkdown; }
set { descriptionMarkdown = value; OnPropertyChanged(); }
set
{
descriptionMarkdown = value?
.Replace("\t", "\u00A0\u00A0\u00A0\u00A0")
.Replace(" ", "\u00A0\u00A0\u00A0\u00A0");
Comment on lines +66 to +68
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The replacement values "\u00A0\u00A0\u00A0\u00A0" are magic strings that appear multiple times without explanation. The decision to replace tabs and four spaces with exactly four non-breaking spaces is not documented. Consider extracting these as named constants with clear documentation explaining why this specific replacement is necessary for fixing the white background issue.

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +68
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The string.Replace method operates sequentially, which can lead to unintended behavior. When " " (four spaces) is replaced with four non-breaking spaces, if the original text contains sequences of more than four spaces, each group of four consecutive spaces will be replaced independently. For example, eight consecutive spaces would become eight non-breaking spaces (correct), but six consecutive spaces would become four non-breaking spaces followed by two regular spaces (inconsistent). Consider replacing multiple spaces in a single operation or using a more comprehensive approach to handle all space sequences consistently.

Copilot uses AI. Check for mistakes.

OnPropertyChanged();
}
}

public string? NotesMarkdown
{
get { return notesMarkdown; }
set { notesMarkdown = value; OnPropertyChanged(); }
set
{
notesMarkdown = value?
.Replace("\t", "\u00A0\u00A0\u00A0\u00A0")
.Replace(" ", "\u00A0\u00A0\u00A0\u00A0");
Comment on lines +79 to +81
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The whitespace replacement logic is duplicated in both DescriptionMarkdown and NotesMarkdown setters. This violates the DRY principle and makes maintenance more difficult. If the replacement logic needs to be changed in the future, it will need to be updated in both places. Consider extracting this logic into a private helper method.

Copilot uses AI. Check for mistakes.

OnPropertyChanged();
}
}
public string CloudSaveMatchTitle
{
Expand Down