project-system icon indicating copy to clipboard operation
project-system copied to clipboard

Clean up project file editing experience

Open tmeschter opened this issue 3 years ago • 0 comments

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 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.

tmeschter avatar Jun 27 '22 20:06 tmeschter