Add "UTF8BOM" and "UTF8NoBOM" to the acceptable values for Encoding parameter.

[mkht]

Pull Request (PR) description

Added UTF8BOM and UTF8NoBOM to the acceptable values for Encoding parameter to the KeyValuePairFile and ReplaceText.

This Pull Request (PR) fixes the following issues

• Fixes Add ability to specify whether UTF8 files have a BOM or not #56

Task list

• Added an entry under the Unreleased section of the change log in the CHANGELOG.md.
  Entry should say what was changed, and how that affects users (if applicable).
• Resource parameter descriptions added/updated in schema.mof
  and comment-based help.
• Comment-based help added/updated.
• Unit tests added/updated. See DSC Resource Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Resource Testing Guidelines.
• New/changed code adheres to DSC Resource Style Guidelines and Best Practices.

---

This change is 

[PlagueHO]

Hi @mkht - can you resolve conflicts and I'll review? Sorry about long delay.

[mkht]

I am surprised and pleased to see this project back in action.

We still use PowerShell DSC and this module almost every day. 😄

I would be happy to have you review it.

[johlju]

Can you change this line

FileContentDsc/azure-pipelines.yml

Line 30 in 0d1d026

dotnet tool install --global GitVersion.Tool

To:

dotnet tool install --global GitVersion.Tool --version 5.*

It will make the build pipeline work. There is a breaking change in v6 that is not yet supported by the pipeline.

[codecov]

Codecov Report

Attention: Patch coverage is 89.70588% with 7 lines in your changes missing coverage. Please review.

Project coverage is 95%. Comparing base (0d1d026) to head (d30f23c).

Files with missing lines	Patch %	Lines
...s/FileContentDsc.Common/FileContentDsc.Common.psm1	92%	3 Missing ⚠️
...ces/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1	88%	2 Missing ⚠️
.../DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1	80%	2 Missing ⚠️

Additional details and impacted files

@@        Coverage Diff         @@
##           main   #59   +/-   ##
==================================
- Coverage    95%   95%   -1%     
==================================
  Files         4     4           
  Lines       264   304   +40     
==================================
+ Hits        253   290   +37     
- Misses       11    14    +3     

Files with missing lines	Coverage Δ
...ces/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1	95% <88%> (-2%)	⬇️
.../DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1	97% <80%> (+2%)	⬆️
...s/FileContentDsc.Common/FileContentDsc.Common.psm1	88% <92%> (+5%)	⬆️

[PlagueHO]

Thank you for your patience @mkht - sadly I get almost no time on DSC anymore, but I try 😁

Reviewed 5 of 10 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status: 6 of 11 files reviewed, 12 unresolved discussions (waiting on @mkht)

---

CHANGELOG.md line 8 at r3 (raw file):

## [Unreleased]

### Changed

Can you also add the change to the buildpipeline GitVersion.Tool to v5?

---

CHANGELOG.md line 26 at r3 (raw file):

- Automatically publish documentation to GitHub Wiki - Fixes [Issue #51](https://github.com/dsccommunity/FileContentDsc/issues/51).
- Renamed `master` branch to `main` - Fixes [Issue #53](https://github.com/dsccommunity/FileContentDsc/issues/53).
- Added `UTF8BOM` and `UTF8NoBOM` for Encoding parameter of the KeyValuePairFile and ReplaceText - Fixes [Issue #56](https://github.com/dsccommunity/FileContentDsc/issues/56).

nit:

Suggestion:

- Added `UTF8BOM` and `UTF8NoBOM` options to the Encoding parameter of the KeyValuePairFile and ReplaceText - Fixes [Issue #56](https://github.com/dsccommunity/FileContentDsc/issues/56).

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 201 at r3 (raw file):

    Assert-ParametersValid @PSBoundParameters

    $fileEncoding = Get-FileEncoding $Path -ErrorAction SilentlyContinue

I notice this code block is almost identical to the code block in Test-TargetResource. (except this one uses ErrorAction SilientlyContinue). I'm wondering if:

1. We shouldn't remove the ErrorAction SilentlyContinue (not sure why I did this at all - it worries me a bit).
2. Refactor this into a Get-FileContent function to reduce code duplication but also make this easier to test.

What do you think?

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 274 at r3 (raw file):

            {
                if ($PSBoundParameters.ContainsKey('Encoding') -and `
                    (($Encoding -eq $fileEncoding) -or `

Would it be possible to refactor this code logic into a separate function (like Test-FileCodingEqual or something). This would make it simpler to test?

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 408 at r3 (raw file):

    }

    $fileEncoding = Get-FileEncoding $Path -ErrorAction SilentlyContinue

See comment above about possible reduction in code duplication by refactoring into a new function.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 45 at r3 (raw file):

    $fileEncoding = Get-FileEncoding $Path -ErrorAction SilentlyContinue
    if ($null -eq $fileEncoding)

See comment above about possible reduction in code duplication by refactoring into a new function.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 158 at r3 (raw file):

    $fileEncoding = Get-FileEncoding $Path -ErrorAction SilentlyContinue
    if ($null -eq $fileEncoding)

See comment above about possible reduction in code duplication by refactoring into a new function.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 290 at r3 (raw file):

    $fileEncoding = Get-FileEncoding $Path -ErrorAction SilentlyContinue
    if ($null -eq $fileEncoding)

See comment above about possible reduction in code duplication by refactoring into a new function.

---

source/Modules/FileContentDsc.Common/FileContentDsc.Common.psm1 line 180 at r3 (raw file):

    $ByteParam = if ($PSVersionTable.PSVersion.Major -ge 6)
    {
        @{AsByteStream = $true }

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    AsByteStream = $true
}

---

source/Modules/FileContentDsc.Common/FileContentDsc.Common.psm1 line 184 at r3 (raw file):

    else
    {
        @{Encoding = 'byte' }

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    Encoding = 'byte'
}

---

tests/TestHelpers/CommonTestHelper.psm1 line 115 at r3 (raw file):

    $ByteParam = if ($PSVersionTable.PSVersion.Major -ge 6)
    {
        @{AsByteStream = $true }

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    AsByteStream = $true
}

---

tests/TestHelpers/CommonTestHelper.psm1 line 119 at r3 (raw file):

    else
    {
        @{Encoding = 'byte' }

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    Encoding = 'byte'
}

[mkht]

Reviewable status: 3 of 11 files reviewed, 11 unresolved discussions (waiting on @PlagueHO)

---

CHANGELOG.md line 8 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you also add the change to the buildpipeline GitVersion.Tool to v5?

Done.

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 201 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

I notice this code block is almost identical to the code block in Test-TargetResource. (except this one uses ErrorAction SilientlyContinue). I'm wondering if:

1. We shouldn't remove the ErrorAction SilentlyContinue (not sure why I did this at all - it worries me a bit).
2. Refactor this into a Get-FileContent function to reduce code duplication but also make this easier to test.

What do you think?

Done.

Refactored to Get-FileContent, leaving SilentlyContinue in place.
It seems to me that SilentlyContinue was specified to avoid an error if the file does not exist. It might be better to be consistent since there are places in the code where the file is checked for existence in advance and other places where it is not.

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 274 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Would it be possible to refactor this code logic into a separate function (like Test-FileCodingEqual or something). This would make it simpler to test?

Done.

---

source/DSCResources/DSC_KeyValuePairFile/DSC_KeyValuePairFile.psm1 line 408 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

See comment above about possible reduction in code duplication by refactoring into a new function.

Done.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 45 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

See comment above about possible reduction in code duplication by refactoring into a new function.

Done.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 158 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

See comment above about possible reduction in code duplication by refactoring into a new function.

Done.

---

source/DSCResources/DSC_ReplaceText/DSC_ReplaceText.psm1 line 290 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

See comment above about possible reduction in code duplication by refactoring into a new function.

Done.

---

source/Modules/FileContentDsc.Common/FileContentDsc.Common.psm1 line 180 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    AsByteStream = $true
}

Done.

---

source/Modules/FileContentDsc.Common/FileContentDsc.Common.psm1 line 184 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    Encoding = 'byte'
}

Done.

---

tests/TestHelpers/CommonTestHelper.psm1 line 115 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    AsByteStream = $true
}

Done.

---

tests/TestHelpers/CommonTestHelper.psm1 line 119 at r3 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you adjust the formatting of this hashtable to fix the HQRM tests. It should be:

@{
    Encoding = 'byte'
}

Done.