-
Notifications
You must be signed in to change notification settings - Fork 43
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
XDT creates inconsistent line endings #137
Comments
I'm not sure about uninstall/cleanup - but I would be much happier with a build task making the changes to web.config/app.config based on the current package version, and if required then adding some documented properties that can be included in the project file. |
We obviously use build tasks to copy the roslyn binaries into the output. But web/app.config updates are a little different. If registering the providers with default options was the only concern, then a build task would be the obvious way to go. I think this was part of the idea behind packageReference getting rid of install.ps1 and XDT transforms - that nuget packages should only be distributing libraries and content. But they did not consider packages that distribute updateable content. The scenario here partly falls into that updatable category. People want to customize the options these providers are configured with. The fragile things we do in install.ps1 to account for customization will also be fragile in an MSBuild task, except they will now be fragile on every build and not immediately visible/fixable in the IDE. I'm not absolutely opposed to that approach, and have considered it before. But it's not at all straightforward or any more reliable. And quite frankly, I don't feel 100% comfortable with how much time I can put into it or how many resources we have on the team for testing all the various scenarios to make sure we're not screwing anybody up. I go back and forth on whether it's worth it. If the convenience of not having to manually update your config file once is worth the work and potential reliability/diagnosability risk. Although, I actually filed this issue to have a public note of why folks might be seeing VS dialogs about fixing line endings. The install.ps1 vs .targets debate is tangentially lurking in the shadows. 😈 |
Seems that in some circumstances, the XDT transforms used upon package install/uninstall can leave inconsistent
line endings in the config file which results in a dialog being popped up by VS which isn't a great experience. Especially
since that dialog doesn't give much insight into why the line endings are inconsistent. (The scenario is when there are
multi-line comments in the config file btw.)
When we write to the config file with our install.ps1 script, it re-normalizes all the line endings so there isn't any warning
upon install. But when uninstalling (upgrading too?) our uninstall.ps1 runs first and the XDT after, so we can't inadvertently
fix the issue.
One approach could be to abandon our XDT's entirely and do that work in our .ps1 scripts. That might be more fragile
though, and I'm not sure this crops up often enough or meets the annoying-enough threshold for that risk.
The other thing to consider here is that neither XDT transforms nor un/install.ps1 scripts are supported in packageReference
projects, so I'm not inclined to spend lots of resources to fix a scenario that is increasingly not recommended. (I don't know
that there is a good strategy for handling these projects though. Running a build task to transform config if it does not see
providers registered already? That doesn't help discoverability for anyone who wants to customize any settings. And it doesn't
help us with a path to migrate previous customizations forward if any options change between versions. It's really a sad
story that we have to put this burden on the developer when then install our package in packageReference projects... but
I don't know that I want to make it even more confusing by half-handedly doing some default things sometimes in a
non-obvious behind-the-scenes sort of way.)
The text was updated successfully, but these errors were encountered: