GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — No longer skips a column without name if double quote delimiter is used in Import-Csv by Topping · Pull Request #7899 · PowerShell/PowerShell

Conversation

@Topping

@Topping Topping commented Oct 1, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

PR Summary

Import-Csv no longer skips double quotation marks, when using " as delimiter, as referenced in #7110

Added tests for possible regressions.

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • User-facing Documentation needed
      • Issue filed - Issue link:
  • Testing - New and feature
    • Not Applicable or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vexx32 and 1RedOne reacted with hooray emoji
All reactions
  • 🎉 2 reactions
Topping added 2 commits October 1, 2018 02:07
@Topping
Fixed handling of double quotes with quote delimiter
445fc77
@Topping
Cleaned whitespace
0916f41
@Topping
Topping requested review from James Truher (JamesWTruher) and Paul Higinbotham (PaulHigin) as code owners October 1, 2018 00:14
@msftclas

Microsoft Contribution License Agreements (msftclas) commented Oct 1, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Topping Topping closed this Oct 1, 2018
@Topping Topping reopened this Oct 1, 2018
iSazonov
Ilya (iSazonov) reviewed Oct 1, 2018
View reviewed changes
Comment thread ...shell/Modules/Microsoft.PowerShell.Utility/assets/TestImportCsvQuoteDelimiter_EmptyValue.csv Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.
@iSazonov
Ilya (iSazonov) requested a review from Steve Lee (SteveL-MSFT) October 1, 2018 10:02
@iSazonov Ilya (iSazonov) self-assigned this Oct 1, 2018
Topping added 2 commits October 1, 2018 21:00
@Topping
Refactored test to use -TestCases
0e9a0a6 
Renamed to clarify purpose of testing with double quotes
@Topping
Added newline to all test files
d195c63
@iSazonov Ilya (iSazonov) changed the title Double quote delimiter import csv issue 7110 No longer skips double quote delimiter in Import-Csv Oct 2, 2018
iSazonov
Ilya (iSazonov) approved these changes Oct 2, 2018
View reviewed changes
@iSazonov Ilya (iSazonov) added the Hacktoberfest Potential candidate to participate in Hacktoberfest label Oct 2, 2018
SteveL-MSFT
Steve Lee (SteveL-MSFT) requested changes Oct 3, 2018
View reviewed changes
Comment thread ...shell/Modules/Microsoft.PowerShell.Utility/assets/TestImportCsvQuoteDelimiter_EmptyValue.csv Outdated
@@ -0,0 +1,2 @@
a1""a3

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 3, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Would prefer to generate these at runtime than have it checked in as it's easier to see the test file in the test code than open another file. Something like:

$emptyValueCsv = @'
a1""a3
v1"v2"v3
'@

Set-Content testdrive:/EmptyValue.csv -Value $emptyValueCsv

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@Topping Topping Oct 3, 2018

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I looked into using a here-string earlier, but the lack of indentation for the string terminator made the code look quite messy.

I couldn't quite find an elegant way to add the here-string, but I've gone an added one for the different file contents.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Outdated
}

It "Should handle <name>" -TestCases @(
@{ name = "quote with empty value"; expectedHeader = "a1,H1,a3"; file = $TestImportCsvQuoteDelimiter_EmptyValue; delimiter = '"' }

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 3, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This formatting seems off, lines 33-38 have one extra indentation. Also, it's nice to align the semicolons in the hashtable per row to make them easier to read

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Topping Topping Oct 3, 2018

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Turns out those lines had tabs as indentation, instead of spaces like the rest of the lines.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Topping added 3 commits October 3, 2018 20:07
@Topping
Added import-csv.tests content inline in the test.
9f8107b 
Content for double quote delimiter test moved inside the test file.
@Topping
Added indentation to the here-string in import-csv
38436ca 
Indented test file content for double quote tests
@Topping
Replaced erroneous tabs with spaces
082e80f 
Few lines in import-csv.tests had tabs for indentation instead of spaces.
SteveL-MSFT
Steve Lee (SteveL-MSFT) requested changes Oct 3, 2018
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1 Resolved

Uh oh!

There was an error while loading. Please reload this page.
iSazonov
Ilya (iSazonov) reviewed Oct 4, 2018
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Import-Csv.Tests.ps1
@{ name = "value enclosed in quote" ; expectedHeader = "a1,a2,a3"; file = "QuotedCharacter.csv" ; content = $quotedCharacterCsv ; delimiter = ',' }
){
param($expectedHeader, $file, $content, $delimiter)
Set-Content testdrive:/$file -Value $content

@iSazonov Ilya (iSazonov) Oct 4, 2018

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Usualy we do this in BeforeAll block. Then we could pass only file name to the block.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SteveL-MSFT Steve Lee (SteveL-MSFT) Oct 4, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Having it in BeforeAll makes sense if multiple tests use the same test content, but in this case only this test uses them so I think it's ok to just have it here closer to the test code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
SteveL-MSFT
Steve Lee (SteveL-MSFT) approved these changes Oct 4, 2018
View reviewed changes
@iSazonov
Ilya (iSazonov) merged commit 20b2e3d into PowerShell:master Oct 5, 2018
@iSazonov Ilya (iSazonov) changed the title No longer skips double quote delimiter in Import-Csv No longer skips a column without name if double quote delimiter is used in Import-Csv Oct 5, 2018
@iSazonov

Ilya (iSazonov) commented Oct 5, 2018

Copy link
Copy Markdown
Collaborator

Topping Thanks for your contribution!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Aditya Patwardhan (adityapatwardhan) pushed a commit to adityapatwardhan/PowerShell that referenced this pull request Apr 9, 2019
@Topping @iSazonov
No longer skips a column without name if double quote delimiter is us…
0352745 
…ed in Import-Csv (PowerShell#7899)
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@Topping @iSazonov
No longer skips a column without name if double quote delimiter is us…
48b730b 
…ed in Import-Csv (PowerShell#7899)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@PaulHigin Paul Higinbotham (PaulHigin) Awaiting requested review from PaulHigin

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@iSazonov Ilya (iSazonov)

Labels

Hacktoberfest Potential candidate to participate in Hacktoberfest

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Topping @msftclas @iSazonov @SteveL-MSFT
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.