[Hosts] Add functionality to remove leading whitespace #37852

blakestack · 2025-03-09T23:07:01Z

Summary of the Pull Request

Add functionality to remove leading whitespace:

Add a new bool option
Implement the UI for the option in settings app
Expose the new option to the Hosts app
Use the new option
Add test coverage for scenarios where option is on/off

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

To resolve merge conflicts

htcfreek · 2025-03-10T07:10:25Z

@blakestack
Something strange is happening in your PR. Why are all these null files and the .prettoerrc file?

blakestack · 2025-03-10T20:15:09Z

@htcfreek I think these null files are from when I tried to compile PowerToys locally, I accidentally left them in the PR along with the .prettierrc file. I will delete the draft and make a new draft PR. Apart from the null files, am I on the right track with the new feature?

htcfreek · 2025-03-10T20:22:45Z

@blakestack
Deleting the files and pushing the changes should be enough. No need to create a new PR.

Why using a drop-down box? I think an on/off toggle is the better choice.

Resolve merge conflicts

blakestack · 2025-03-10T20:39:48Z

@htcfreek Thank you, I will change it to a on/off toggle and then try to add tests

htcfreek · 2025-03-11T00:29:20Z

src/modules/Hosts/Hosts/Settings/UserSettings.cs

        public UserSettings()
        {
            _settingsUtils = new SettingsUtils();
            ShowStartupWarning = true;
            LoopbackDuplicates = false;
            AdditionalLinesPosition = HostsAdditionalLinesPosition.Top;
            Encoding = HostsEncoding.Utf8;
+            AddLeadingWhitespace = false;


Isn't the current behavior "true"? We shouldn't change that.

@htcfreek I will make these changes now

htcfreek · 2025-03-11T00:35:32Z

src/settings-ui/Settings.UI.Library/HostsProperties.cs

@@ -29,6 +32,7 @@ public HostsProperties()
            ShowStartupWarning = true;
            LaunchAdministrator = true;
            LoopbackDuplicates = false;
+            AddLeadingWhitespace = false;


same: true and not false

htcfreek · 2025-03-11T00:40:04Z

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs

+                        else if (anyDisabled) && (AddLeadingWhitespace == false)                                                               
+                        {                                                                                                                       
+                            lineBuilder.Append(e.line);


Wondering if this case simply should do nothing and is wrong/obsolete.

I got confused with the logic here

the case is obsolete. I was thinking the program would hit the more restrictive condition first, and then adjust for the AddLeadingWhitespace value

htcfreek · 2025-03-11T00:42:25Z

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs

+                        {                                                                                                                       
+                            lineBuilder.Append(e.line);
+                        }                                                                                                                        
+                        else if (anyDisabled)                                                                                                   


If the case above is obsolete you can use & !AddLeadingWhitespace here.

I will change this and then push the changes now

…update the behavior for adding or removing the Whitespace

blakestack · 2025-03-11T01:27:05Z

@htcfreek I made the most recent changes. I am going to try compiling the program and see if it works.

…g whitespaces from the code

blakestack · 2025-03-11T02:16:27Z

I got the program to build, the console gave an error that said the AddLeadingWhite instance variable should be changed to private in the HostsService.cs file. There was also another error that I left trailing whitespaces in the code after deleting my comments, so I deleted the trailing whitespaces. I am not sure how to view the rest of the changes after finishing the build process.

Jay-o-Way · 2025-03-11T15:04:59Z

I think an on/off toggle is the better choice.

In general, Toggle Switches are very often misused. They are designed to indicate an explicit ON and OFF state, as in turning a light on or off. The light stands for ACTION or WORK. Button, checkbox, or maybe a dropdown should be used for other cases. Therefore I say a toggleswitch is not appropriate for this situation.

htcfreek · 2025-03-11T15:07:21Z

I think an on/off toggle is the better choice.

In general, Toggle Switches are very often misused. They are designed to indicate an explicit ON and OFF state, as in turning a light on or off. The light stands for ACTION or WORK. Button, checkbox, or maybe a dropdown should be used for other cases. Therefore I say a toggleswitch is not appropriate for this situation.

Why? We are turning white spaces on or off.

Copilot

Pull Request Overview

This PR adds functionality to remove leading whitespace from hosts entries by introducing a new boolean option.

Adds the new "AddLeadingWhitespace" option in the settings UI and hosts properties.
Integrates the option into the HostsService behavior for writing files.
Adds test coverage to verify toggling the new whitespace removal functionality.

Reviewed Changes

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

Show a summary per file

File	Description
src/settings-ui/Settings.UI/ViewModels/HostsViewModel.cs	Added a new property to expose the AddLeadingWhitespace setting.
src/settings-ui/Settings.UI.Library/HostsProperties.cs	Introduced the new property with JSON conversion and default initialization.
src/modules/Hosts/HostsUILib/Helpers/HostsService.cs	Declared a new field for AddLeadingWhitespace and used it in WriteAsync, but the field is never initialized from user settings.
src/modules/Hosts/Hosts/Settings/UserSettings.cs	Added the new property with default value true in user settings initialization.
src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs	Added a test case to validate the whitespace removal behavior, though the mock instance appears to be mismatched.

Files not reviewed (1)

src/settings-ui/Settings.UI/SettingsXAML/Views/HostsPage.xaml: Language not supported

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs

src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs

Co-authored-by: Copilot <[email protected]>

…Settings' in the constructor to ensure that the value reflects the users setting

resolve merge conflicts

…featureBranch

blakestack · 2025-03-31T21:55:49Z

@htcfreek I just made some changes to my PR. I made the changes that were suggested by copilot. Should I continue with the toggle switch implementation?

htcfreek · 2025-04-02T07:11:17Z

@blakestack
Yes. But I think it makes sense to wait for the PR decision.

crutkas · 2025-05-15T02:41:59Z

/azp run

azure-pipelines · 2025-05-15T02:42:08Z

Azure Pipelines successfully started running 1 pipeline(s).

blakestack · 2025-05-19T17:21:00Z

I checked the errors from the azure-pipelines bot. I made an additional change to make the 'AddLeadingWhitespace' variable public on line 44 of the HostService.cs file. I believe that should resolve all the errors.

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs

Resolve merge conflicts

github-actions · 2025-07-02T16:38:58Z

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (39)

advancedpaste
appxpackage
Ashcraft
CDPX
commandnotfound
copyable
Corpor
cropandlock
environmentvariables
fileexploreraddons
filelocksmith
findmymouse
fucntion
fuzzingtesting
hostsfileeditor
hotfixes
IDOn
lcl
LIBFUZZER
makepri
mikeclayton
mousehighlighter
mousejump
mousepointer
mouseutils
MVPs
onebranch
PMs
Psr
quickaccent
regsvr
screenruler
sharpfuzz
sourced
stuttery
textextractor
Windowss
XLoc
zonability

These words are not needed and should be removed

cleanmgr CLSCTXINPROCALL CLSCTXLOCALSERVER FILELOCKSMITH IIDI iwr psexec smileys TEXTEXTRACTOR windowsterminal Zhiwei

Some files were automatically ignored 🙈

These sample patterns would exclude them:

^src/common/CalculatorEngineCommon/exprtk\.hpp$

You should consider adding them to:

.github/actions/spell-check/excludes.txt

File matching is via Perl regular expressions.

To check these files, more of their words need to be in the dictionary than not. You can use patterns.txt to exclude portions, add items to the dictionary (e.g. by adding them to allow.txt), or fix typos.

To accept these unrecognized words as correct, update file exclusions, and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the [email protected]:blakestack/PowerToys.git repository
on the featureBranch branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/c635c2f3f714eec2fcf27b643a1919b9a811ef2e/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/16030752704/attempts/1' &&
git commit -m 'Update check-spelling metadata'

Forbidden patterns 🙅 (2)

In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves.

These forbidden patterns matched content:

Do not use `(click) here` links

For more information, see:

(?i)(?:>|\[)(?:(?:click |)here|this(?=\]\([^\)]+:/)|link|(?:read |)more(?!</value))(?:</|\]\()

Should be `greater than`

\bhigher than\b

Pattern suggestions ✂️ (1)

You could add these patterns to .github/actions/spell-check/patterns.txt:

# Automatically suggested patterns

# hit-count: 1 file-count: 1
# curl arguments
\b(?:\\n|)curl(?:\.exe|)(?:\s+-[a-zA-Z]{1,2}\b)*(?:\s+-[a-zA-Z]{3,})(?:\s+-[a-zA-Z]+)*

Alternatively, if a pattern suggestion doesn't make sense for this project, add a #
to the beginning of the line in the candidates file with the pattern to stop suggesting it.

Errors, Warnings, and Notices ❌ (4)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors, Warnings, and Notices	Count
ℹ️ candidate-pattern	1
❌ check-file-path	20
❌ forbidden-pattern	2
⚠️ large-file	1

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

binary file (or some other file you wouldn't want to check at all).

Please add a file path to the excludes.txt file matching the containing file.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).
well-formed pattern.

If you can write a pattern that would match it,
try adding it to the patterns.txt file.

Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

Note that patterns can't match multiline strings.

blakestack · 2025-07-02T19:41:28Z

Is there any way to skip the spellcheck job?

htcfreek · 2025-07-02T20:53:48Z

Is there any way to skip the spellcheck job?

@blakestack
You can add the false positives to the expected words list. But first you should merge main in.

https://github.com/microsoft/PowerToys/blob/main/.github%2Factions%2Fspell-check%2Fexpect.txt

crutkas · 2025-07-08T02:43:22Z

/azp run

azure-pipelines · 2025-07-08T02:43:31Z

Azure Pipelines successfully started running 1 pipeline(s).

blakestack added 2 commits March 9, 2025 18:56

Add functionality for removing leading whitespace

59a0954

Merge branch 'main' into featureBranch

f625e2d

To resolve merge conflicts

This comment has been minimized.

Sign in to view

blakestack added 2 commits March 10, 2025 16:31

Delete null files and remove comments

9df30cf

Merge branch 'main' into featureBranch

e86637d

Resolve merge conflicts

Change feature to a toggle switch and add a test case

6980c0e

This comment has been minimized.

Sign in to view

Fix spelling error

b54c40e

htcfreek reviewed Mar 11, 2025

View reviewed changes

Update the default value for the 'AddLeadingWhitespace' variable and …

feab345

…update the behavior for adding or removing the Whitespace

Change 'AddLeadingWhitespace' variable to private, and remove trailin…

f36c45d

…g whitespaces from the code

yeelam-gordon requested a review from Copilot March 31, 2025 05:01

Copilot AI reviewed Mar 31, 2025

View reviewed changes

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs Outdated Show resolved Hide resolved

src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs Outdated Show resolved Hide resolved

blakestack and others added 4 commits March 31, 2025 17:31

Update src/modules/Hosts/Hosts.Tests/HostsServiceTest.cs

31a560a

Co-authored-by: Copilot <[email protected]>

initialize the 'addLeadingWhitespace' variable from the passed 'IUser…

bb6193b

…Settings' in the constructor to ensure that the value reflects the users setting

Merge remote-tracking branch 'upstream/main' into featureBranch

5ddc992

resolve merge conflicts

Merge branch 'featureBranch' of github.com:blakestack/PowerToys into …

a4b50d7

…featureBranch

blakestack marked this pull request as ready for review April 1, 2025 01:41

blakestack requested a review from htcfreek April 2, 2025 00:21

jamrobot changed the title ~~Add functionality to remove leading whitespace~~ [Hosts] Add functionality to remove leading whitespace Apr 8, 2025

jamrobot added the Product-Hosts File Editor Refers to the Hosts file editor label Apr 8, 2025

jamrobot assigned jamrobot and chenmy77 Apr 10, 2025

jamrobot requested a review from chenmy77 April 10, 2025 05:54

jamrobot added the Needs-Review This Pull Request awaits the review of a maintainer. label Apr 10, 2025

htcfreek approved these changes Apr 22, 2025

View reviewed changes

chenmy77 added the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label Apr 25, 2025

chenmy77 added this to the PowerToys 0.92 milestone Apr 25, 2025

crutkas removed the Don't merge - Hold for release Hold off on merging this PR, even if it's approved. label May 15, 2025

Change the 'AddLeading Whitespace' field to 'public'

b6284cd

cinnamon-msft modified the milestones: PowerToys 0.92, PowerToys 0.93 Jun 6, 2025

chenmy77 reviewed Jun 10, 2025

View reviewed changes

src/modules/Hosts/HostsUILib/Helpers/HostsService.cs Show resolved Hide resolved

Define 'AddLeadingWhitespace' variable in the 'IUserSettings.cs' file

3f6d900

This comment has been minimized.

Sign in to view

Merge branch 'main' into featureBranch

ffc8e53

Resolve merge conflicts

crutkas mentioned this pull request Jul 10, 2025

[Hosts] Adds no whitespace option(#36386) #38248

Open

11 tasks

[Hosts] Add functionality to remove leading whitespace #37852

Are you sure you want to change the base?

[Hosts] Add functionality to remove leading whitespace #37852

Uh oh!

Conversation

blakestack commented Mar 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

This comment has been minimized.

htcfreek commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blakestack commented Mar 10, 2025

Uh oh!

htcfreek commented Mar 10, 2025

Uh oh!

blakestack commented Mar 10, 2025

Uh oh!

This comment has been minimized.

htcfreek Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

blakestack Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

htcfreek Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

htcfreek Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

blakestack Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

htcfreek Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

blakestack Mar 11, 2025

Choose a reason for hiding this comment

Uh oh!

blakestack commented Mar 11, 2025

Uh oh!

blakestack commented Mar 11, 2025

Uh oh!

Jay-o-Way commented Mar 11, 2025

Uh oh!

htcfreek commented Mar 11, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

blakestack commented Mar 31, 2025

Uh oh!

htcfreek commented Apr 2, 2025

Uh oh!

crutkas commented May 15, 2025

Uh oh!

azure-pipelines bot commented May 15, 2025

Uh oh!

blakestack commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

github-actions bot commented Jul 2, 2025

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Do not use (click) here links

Should be greater than

See the 📂 files view, the 📜action log, or 📝 job summary for details.

blakestack commented Mar 9, 2025 •

edited

Loading

htcfreek commented Mar 10, 2025 •

edited

Loading

blakestack Mar 11, 2025 •

edited

Loading

blakestack commented May 19, 2025 •

edited

Loading

Do not use `(click) here` links

Should be `greater than`

htcfreek commented Jul 2, 2025 •

edited

Loading