Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Mlonis fix for issue 165 #555

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

Mlonis fix for issue 165 #555

wants to merge 4 commits into from

Conversation

mrlonis
Copy link

@mrlonis mrlonis commented Dec 5, 2019

Description

This pull request resolves issue #165 .

Changelog

Get-PSScriptInfoString.ps1

  1. Removed the trailing whitespace on line 90.
  2. Refactored lines 96-97 as a singular line of code. This change functions just like the code before but, when $ReleaseNotes is the empty string, it will no longer add an extra newline when the release notes section is being left empty.

Get-ScriptCommentHelpInfoString.ps1

  1. Modified line 50, 54, 62, 72, 82, 89, 97, 104, 109, 114, & 117 to remove the trailing whitespaces when creating the comment help info string.

Update-ScriptFileInfo.ps1

  1. Modified line 399 to no longer add a character return newline sequence when the $requiresString is the empty string.
  2. Modified line 405 to no longer add a character return newline sequence when a script already has Requires -Module statements but the update function is not looking to add any new Requires -Module statements.
  3. Modified line 444 to remove the trailing whitespace after the word DESCRIPTION in the script info.

Matthew Lonisis added 4 commits December 4, 2019 19:30
Removed trailing whitespace on line 90. Refactored lines 96-97 to be a one line code. This enhancement functions like the original code, but now won't add an extra newline if the release notes provided are null or empty.
Removed trailing whitespace from the help info text.
1. Removed trailing whitespace on line 444 (now line 454)
2. Refactored lines 399 to no longer add extra whitespace in the event that $requiredStrings is the empty string
3. Added new code between lines 405 and 406 to fully resolve issue #165
@msftclas
Copy link

msftclas commented Dec 5, 2019

CLA assistant check
All CLA requirements met.


.REQUIREDSCRIPTS$(if ($RequiredScripts) {" $($RequiredScripts -join ',')"})

.EXTERNALSCRIPTDEPENDENCIES$(if ($ExternalScriptDependencies) {" $($ExternalScriptDependencies -join ',')"})

.RELEASENOTES
$($ReleaseNotes -join "`r`n")
.RELEASENOTES$(if ($ReleaseNotes) {`r`n$($ReleaseNotes -join "`r`n")})
Copy link
Author

Choose a reason for hiding this comment

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

This simplifies the code into one line and further prevents the addition of a new line when the release notes section is being left blank or empty.

Comment on lines +399 to +403
if ($requiresStrings.Trim() -ne "") {
$tempContents += "$PSScriptInfoString`r`n`r`n$($requiresStrings -join "`r`n")"
} else {
$tempContents += $PSScriptInfoString
}
Copy link
Author

Choose a reason for hiding this comment

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

This resolves issue #165 by not adding extra newlines if the user did not supply any required modules. If the user does not supply any required modules, Get-RequiresString returns the empty string.

Comment on lines +410 to +415
} elseif (($scriptFileContents[$i + 1] -eq "`r`n" -or $scriptFileContents[$i + 1] -eq "") -and (($i + 1) -lt $scriptFileContents.Count)) {
# This condition will only be met if the line is a Requires -Module statement.
# To adding newlines everytime the function is called on a script, we must increment i by 1
# if the next line after the caught Requires -Module statement is the empty string or a `r`n
# This prevents extra newlines from being inserted
$i = $i + 1
Copy link
Author

Choose a reason for hiding this comment

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

This section accounts for the edge case where a script file already has Requires -Module statements and prevents the addition of new lines by incrementing $i when it detects that the line following the last Requires -Modules statement is a character return newline sequence or the empty string.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants