Skip to content

Option disable literal strings - #245

Merged
EamonNerbonne merged 5 commits into
EamonNerbonne:masterfrom
girlpunk:option-disable-literal-strings
Aug 25, 2022
Merged

EamonNerbonne merged 5 commits into
EamonNerbonne:masterfrom
girlpunk:option-disable-literal-strings

Conversation

@girlpunk

Copy link
Copy Markdown
Contributor

In some environments, such as Dynamic LINQ (System.Linq.Dynamic), literal strings are not supported. As such, an option to disable generating code using these would be useful.

@EamonNerbonne

Copy link
Copy Markdown
Owner

I think the term "verbatim" is more conventional here than "literal". At first glance this looks great. I'll have time to look at this in more detail on thursday (the 25th). Thanks for the contribution anyhow!

@EamonNerbonne
EamonNerbonne merged commit 058c71c into EamonNerbonne:master Aug 25, 2022
@girlpunk
girlpunk deleted the option-disable-literal-strings branch August 25, 2022 21:43
@girlpunk

Copy link
Copy Markdown
Contributor Author

I've seen people use "verbatim", "literal", and "raw" strings in various places, iirc there was already some parts of ExpressionToCode referring to them as "literal", so went for consistency, but I'm happy to get that changed if you'd like

@EamonNerbonne

Copy link
Copy Markdown
Owner

Yeah, it's not just this code. I'll rename that.

Incidentally, I'm currently debating whether I want to release this as-is, or clean up the API. Because one thing that isn't new in thie PR, but is more prominent after this PR is the existance of a public IObjectStringifier. However, in practice, this is just a slightly complicated way of changing the config: it'd be nicer to simply have properties such as this one, but also such as the option to always use fully-qualified type names on the plain config object, and to have that be a record so the "new" with syntax works naturally.

The old design was motivated mostly by the lack of a language feature like records, and by the practicalities of making full-type-name-qualification configurable (in #42). But it's a bit messy.

On the other hand, breaking changes in libs are annoying.

If you want to build on the change in this PR, I could always release a 4.0.0-preview1 version or whatever; that way it's immediately usable without being a default upgrade target for other users, and then I can figure out how best to make that trade-off between a clean API and avoiding breaking changes.

If you have any opinion on the API, feel free to mention em ;-).

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

Successfully merging this pull request may close these issues.

2 participants