search

CommunityToolkit / Labs-Windows

A safe space to collaborate and engineer solutions from the prototyping stage all the way through polished finalized component for the Windows Community Toolkit.
Other
341 stars 47 forks source link

XAML Style check was failing due to the location of the settings file. #138

Open michael-hawker opened 2 years ago

michael-hawker commented 2 years ago

XAML Style check was failing due to the location of the settings file. The default behavior for the extension inside VS is for it only to look for a settings file as far as the directory the solution file is in. The Labs structure puts the global settings file in the root of the repo and so isn't used.
In this location, it won't be used unless a setting is changed in the extension. If Search to drives root is set to true, the extension will find the correct settings and format the XAML as desired.
Without changing this setting, it's likely that issues of XAML formatting in individual experiments are likely to continue to cause CI to fail. :(

Originally posted by @mrlacey in https://github.com/CommunityToolkit/Labs-Windows/issues/137#issuecomment-1144990117

mrlacey commented 2 years ago

Good news: XamlStyler does work with a symlink in the same directory as the solution file that points to the settings file in the repository root.

Bad news: The templating system includes a copy of the linked file in the generated output if a symlink is included in the template source directories.

This leaves a few options:

  1. Include a copy of the settings file in the same dir as the .sln files. (we can still use a symlink in the template so it copies the one from the repo root into newly created experiments.)
  2. Seriously rethink the current directory structure and the location of sln & xamlstyler files. (Although I can't--yet--think of a way around the issue that also works with using templates to generate new experiments.
  3. Add something to forcibly run the ApplyXamlStyling script so people don't see the issue at all and don't have to install the extension. (Maybe as a pre- or post-build step in one of the projects within an experiment.)

@michael-hawker @Arlodotexe Thoughts?

michael-hawker commented 2 years ago

Thanks for taking a look @mrlacey!

I think having the solutions all be in the labs directory outside their respective solutions could be interesting for 2, and then we just put the settings file there. But that's not going to work well with how templates get created (as far as I know). That also means it's not easy to grab those settings in our other sample app project, but I think in that case we can just symlink and should be fine as long as that's preserved when cloning the repo.

I had originally wanted to do the styling as a pre-check-in step, but you can't commit git hooks to a repo, so that didn't work out well. We could try doing it pre-build and see how that works and what that experience is like? I mean the experience should be similar to if you had the extension installed, right? I think the main question there is if it 'touches' the files without changes and forces more rebuilds elsewhere?

Arlodotexe commented 2 years ago
  1. Add something to forcibly run the ApplyXamlStyling script so people don't see the issue at all and don't have to install the extension. (Maybe as a pre- or post-build step in one of the projects within an experiment.)

This could be doable, so long as it affects XAML and not any of the props or csproj files.

mrlacey commented 2 years ago

This could be doable, so long as it affects XAML and not any of the props or csproj files.

I don't know why it would. The script in question explicitly filters on xaml files. Is there something you know about XamlStyler that means it might touch these files?

michael-hawker commented 2 years ago

@grochocki was wondering if you had any suggestions on our approach here or other options we may not be aware of?

michael-hawker commented 2 years ago

There's a look up to root setting here:

image

Enabling that, resolves the issue and makes the projects pick the root definition up.