Skip to content
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

Jump to bottom

Prefer use of interpolated strings in PresentationCore #8616

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Prefer use of interpolated strings in PresentationCore #8616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e6f1f5
use interpolated strings in PresentationCore
halgab Dec 29, 2023
193511f
manual followups
halgab Dec 30, 2023
4ea7631
StringBuilder.AppendFormat refactos
halgab Dec 30, 2023
e7de550
fix Media.Color
halgab Jan 31, 2024
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
8 changes: 2 additions & 6 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/AppModel/SiteOfOriginContainer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@ internal static Uri SiteOfOrigin
#if DEBUG
if (_traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginContainer: returning site of origin " + siteOfOrigin);
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginContainer: returning site of origin {siteOfOrigin}");
Comment on lines 58 to +61
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A follow-up to this is to introduce a Trace method with a custom interpolated string handler that encapsulate all this logic. It would clean-up the code quite a lot

Copy link
Contributor

Choose a reason for hiding this comment

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

@halgab Having an encapsulated function would be beneficial for maintaining clean code. Could you please provide the function's arguments and structure that you are considering?

We are planning to add this PR for this month test-pass. So, can you resolve the merge conflicts for this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a draft PR based on this one here: #8702. Its last commit is what I have in mind. It's based on similar changes I contributed in the winforms repo.

#endif

return siteOfOrigin;
Expand Down Expand Up @@ -229,9 +227,7 @@ protected override PackagePart GetPartCore(Uri uri)
#if DEBUG
if (_traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginContainer: Creating SiteOfOriginPart for Uri " + uri);
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginContainer: Creating SiteOfOriginPart for Uri {uri}");
#endif
return new SiteOfOriginPart(this, uri);
}
Expand Down
36 changes: 9 additions & 27 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/AppModel/SiteOfOriginPart.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,7 @@ protected override Stream GetStreamCore(FileMode mode, FileAccess access)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Getting stream.");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Getting stream.");
#endif
return GetStreamAndSetContentType(false);
}
Expand All @@ -67,9 +65,7 @@ protected override string GetContentTypeCore()
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Getting content type.");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Getting content type.");
#endif

GetStreamAndSetContentType(true);
Expand All @@ -95,9 +91,7 @@ private Stream GetStreamAndSetContentType(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Getting content type and using previously determined value");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Getting content type and using previously determined value");
#endif
return null;
}
Expand All @@ -111,9 +105,7 @@ private Stream GetStreamAndSetContentType(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
"SiteOfOriginPart: Using Cached stream");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}SiteOfOriginPart: Using Cached stream");
#endif
Stream temp = _cacheStream;
_cacheStream = null;
Expand All @@ -125,9 +117,7 @@ private Stream GetStreamAndSetContentType(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Determining absolute uri for this resource");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Determining absolute uri for this resource");
#endif
string original = Uri.ToString();
Invariant.Assert(original[0] == '/');
Expand All @@ -138,9 +128,7 @@ private Stream GetStreamAndSetContentType(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Making web request to " + _absoluteLocation);
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Making web request to {_absoluteLocation}");
#endif

// For performance reasons it is better to open local files directly
Expand All @@ -164,9 +152,7 @@ private Stream HandleFileSource(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": Opening local file " + _absoluteLocation);
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: Opening local file {_absoluteLocation}");
#endif
if (_contentType == MS.Internal.ContentType.Empty)
{
Expand All @@ -188,19 +174,15 @@ private Stream HandleWebSource(bool onlyNeedContentType)
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": Successfully retrieved stream from " + _absoluteLocation);
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: Successfully retrieved stream from {_absoluteLocation}");
#endif

if (_contentType == MS.Internal.ContentType.Empty)
{
#if DEBUG
if (SiteOfOriginContainer._traceSwitch.Enabled)
System.Diagnostics.Trace.TraceInformation(
DateTime.Now.ToLongTimeString() + " " + DateTime.Now.Millisecond + " " +
Environment.CurrentManagedThreadId +
": SiteOfOriginPart: Setting _contentType");
$"{DateTime.Now:T} {DateTime.Now.Millisecond} {Environment.CurrentManagedThreadId}: SiteOfOriginPart: Setting _contentType");
#endif

_contentType = WpfWebRequestHelper.GetContentType(response);
Expand Down
12 changes: 4 additions & 8 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/FontCache/FontCacheUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ internal static string CompositeFontExtension

static Util()
{
string s = Environment.GetEnvironmentVariable(WinDir) + @"\Fonts\";
string s = $@"{Environment.GetEnvironmentVariable(WinDir)}\Fonts\";

_windowsFontsLocalPath = s.ToUpperInvariant();

Expand Down Expand Up @@ -538,7 +538,7 @@ internal static Uri CombineUriWithFaceIndex(string fontUri, int faceIndex)
// so that they don't conflict with the fragment part.
string canonicalPathUri = new Uri(fontUri).GetComponents(UriComponents.AbsoluteUri, UriFormat.SafeUnescaped);
string faceIndexString = faceIndex.ToString(CultureInfo.InvariantCulture);
return new Uri(canonicalPathUri + '#' + faceIndexString);
return new Uri($"{canonicalPathUri}#{faceIndexString}");
}

internal static bool IsSupportedFontExtension(string extension, out bool isComposite)
Expand Down Expand Up @@ -662,7 +662,7 @@ private static string NormalizeFontFamilyReference(string fontFamilyReference, i
{
// No fragment separator. The entire string is a family name so convert to uppercase
// and add a fragment separator at the beginning.
return string.Concat("#", fontFamilyReference.AsSpan(startIndex, length)).ToUpperInvariant();
return $"#{fontFamilyReference.AsSpan(startIndex, length)}".ToUpperInvariant();
}
else if (fragmentIndex + 1 == startIndex + length)
{
Expand Down Expand Up @@ -702,11 +702,7 @@ internal static string ConvertFamilyNameAndLocationToFontFamilyReference(string
if (!string.IsNullOrEmpty(location))
{
// We just escaped the family name and the location part should already be a valid URI reference.
fontFamilyReference = string.Concat(
location,
"#",
fontFamilyReference
);
fontFamilyReference = $"{location}#{fontFamilyReference}";
}

return fontFamilyReference;
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/FontCache/FontSourceCollection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ private void SetFontSources()
{
if (_tryGetCompositeFontsOnly)
{
files = Directory.GetFiles(_uri.LocalPath, "*" + Util.CompositeFontExtension);
files = Directory.GetFiles(_uri.LocalPath, $"*{Util.CompositeFontExtension}");
isOnlyCompositeFontFiles = true;
}
else
Expand Down Expand Up @@ -168,7 +168,7 @@ private void SetFontSources()
{
if (_tryGetCompositeFontsOnly)
{
files = Directory.GetFiles(_uri.LocalPath, "*" + Util.CompositeFontExtension);
files = Directory.GetFiles(_uri.LocalPath, $"*{Util.CompositeFontExtension}");
isOnlyCompositeFontFiles = true;
}
else
Expand Down Expand Up @@ -269,7 +269,7 @@ System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
private bool _tryGetCompositeFontsOnly;

private const string InstalledWindowsFontsRegistryKey = @"SOFTWARE\Microsoft\Windows NT\CurrentVersion\Fonts";
private const string InstalledWindowsFontsRegistryKeyFullPath = @"HKEY_LOCAL_MACHINE\" + InstalledWindowsFontsRegistryKey;
private const string InstalledWindowsFontsRegistryKeyFullPath = $@"HKEY_LOCAL_MACHINE\{InstalledWindowsFontsRegistryKey}";
}
}

2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/FontFace/CompositeFontParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ private void ParseFontFamilyCollectionElement()

if (!foundOsSection)
{
Fail(string.Format("No FontFamily element found in FontFamilyCollection that matches current OS or greater: {0}", OSVersionHelper.GetOsVersion().ToString()));
Fail($"No FontFamily element found in FontFamilyCollection that matches current OS or greater: {OSVersionHelper.GetOsVersion()}");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/Ink/ErasingStroke.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ internal void MoveTo(IEnumerable<Point> path)
}
#if POINTS_FILTER_TRACE
_totalPointsAdded += path.Length;
System.Diagnostics.Debug.WriteLine(String.Format("Total Points added: {0} screened: {1} collinear screened: {2}", _totalPointsAdded, _totalPointsScreened, _collinearPointsScreened));
System.Diagnostics.Debug.WriteLine($"Total Points added: {_totalPointsAdded} screened: {_totalPointsScreened} collinear screened: {_collinearPointsScreened}");
#endif

}
Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.DotNet.Wpf/src/PresentationCore/MS/internal/Ink/ExtendedProperty.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,13 @@ public override string ToString()
}
else if (Value is string)
{
val = "\"" + Value.ToString() + "\"";
val = $"\"{Value}\"";
}
else
{
val = Value.ToString();
}
return KnownIds.ConvertToString(Id) + "," + val;
return $"{KnownIds.ConvertToString(Id)},{val}";
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion ...oft.DotNet.Wpf/src/PresentationCore/MS/internal/Ink/GestureRecognizer/NativeRecognizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ enum RECO_TYPE : ushort
}

private const string GestureRecognizerPath = @"SOFTWARE\MICROSOFT\TPG\SYSTEM RECOGNIZERS\{BED9A940-7D48-48E3-9A68-F4887A5A1B2E}";
private const string GestureRecognizerFullPath = "HKEY_LOCAL_MACHINE" + @"\" + GestureRecognizerPath;
private const string GestureRecognizerFullPath = $@"HKEY_LOCAL_MACHINE\{GestureRecognizerPath}";
private const string GestureRecognizerValueName = "RECOGNIZER DLL";
private const string GestureRecognizerGuid = "{BED9A940-7D48-48E3-9A68-F4887A5A1B2E}";

Expand Down
Loading
Loading