dotnet / dotnet/project-system

Clean up project file editing experience

Open
#8,271 0 comments 1 reaction 0 assignees View on GitHub
Triage-Approved
Dominant language
C#
Stars
1k
Forks
415
PR merge metrics
No merged PRs in 30d

Description

For any given MSBuild file (whether that be a project file, the .user file, or a directly- or indirectly-imported .props/.targets) the file data can exist in up to three different structures:
- On disk
- In a VS text buffer
- In the MSBuild data model

Due to editing of the data, it is possible that each of these structures has different content for the "same" file. Historically this has been a source of bugs when the structures would move out-of-sync and change made through the MSBuild data model (for example) would clobber changes made through the text buffer, or when changes made through the MSBuild data model were never persisted to the text buffer or file on disk (causing those changes to be silently lost when the project was unloaded).

Compounding this, CPS really only manages the project file itself, and the associated .user file (if any). It doesn't have a general mechanism to handle changes to an imported .props or .targets file made through the editor, or to ensure changes made through the MSBuild data model are persisted back to the open editor for that file. Parts of CPS that may be modifying a .props or .targets file currently need to explicitly save the data model back to the file on disk or they are likely to be lost. A file may not show that it has been edited even though it has been.

We need to strength the CPS understanding of imported .props/.targets files and rationalize the three-way merge of on-disk, in-buffer, and in-data-model project data in order to avoid further weird bugs _and_ in order to support features that modify these files, like Central Package Management and an MSBuild language service.

Examples of where this has been a problem:

- The support for Central Package Management in [CPS PR 404869](https://devdiv.visualstudio.com/DevDiv/_git/CPS/pullrequest/404869) had to work around the fact that CPS really only fully understands changes to the project and .user file, and doesn't know to persist other changes to disk/buffer.
- #5429 may be fall out from the above issues, or may be a separate compounding problem. As part of this work we should make sure this scenario is addressed.

Other things to consider:
- Anywhere CPS may need to modify an MSBuild file through the MSBuild API (e.g. when a Directory.Packages.props file is updated) we should make sure the file was first loaded into the MSBuild data model with the `PreserveFormat` flag; otherwise we risk nuking comments and formatting when the changes are saved back to disk. This is something Lifeng has mentioned as a _possible_ issue and it is currently not clear how big a problem it actually is.
- Editor tracking state (I'm not entirely sure what this means; I might mean making sure undo/redo works correctly in the face of edits via the MSBuild API, or it may mean that when editing a file manually we should "push" changes to the MSBuild data model whenever the file is in a valid state).
- Everywhere we use the MSBuild API to modify a file we should be using the recently-introduced CPS service to check if a file is considered "editable" or not.
- That same "editable file" service in the previous point needs to be made into a public contract so others can implement it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.