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.

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

CsvContext.Configuration.ShouldQuote is not used #2292

Open
gmkado opened this issue Oct 4, 2024 · 2 comments
Open

CsvContext.Configuration.ShouldQuote is not used #2292

gmkado opened this issue Oct 4, 2024 · 2 comments

Comments

@gmkado
Copy link

gmkado commented Oct 4, 2024

I'm using v30.0.1 but I believe this is true for the latest.

using var writer = new StreamWriter(_csvStream, Encoding.Default, BufferSize, true);
using var csvWriter = new CsvWriter(writer, CultureInfo.InvariantCulture);
csvWriter.Context.Configuration.ShouldQuote = _ => false;

This seems to use the ConfigurationFunctions.ShouldQuote instead of my delegate, I believe because it gets copied here locally on construction of CsvWriter:

shouldQuote = configuration.ShouldQuote;

I'm updating from v15 to v30, so I'm still trying to figure out all the changes. Is the Context.Configuration meant to expose mutable fields on the writer's configuration? If so, should the WriteField method use configuration.ShouldQuote instead of storing a local copy on construction?

I understand I can set the configuration and pass it to CsvWriter, but seems like Context.Configuration.ShouldQuote should not be settable if its not intended to take effect.

@gmkado gmkado changed the title CsvContext.ConfigurationShouldQuote is not used CsvContext.Configuration.ShouldQuote is not used Oct 4, 2024
@AltruCoder
Copy link

The only way to set those values is to pass a CsvConfiguration object to the CsvWriter constructor.

Changes were made in v20 to make CsvConfiguration readonly. https://joshclose.github.io/CsvHelper/change-log/#section-52

Changed CsvConfiguration to a read only record to eliminate threading issues.

It looks like copying the config values to a local private copy after passing CsvConfiguration to the constructor was also done because of threading issues. #1623 (comment)

@gmkado
Copy link
Author

gmkado commented Oct 9, 2024

Would it make sense then for CsvContext to expose a read-only version of the configuration?

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

No branches or pull requests

2 participants