Skip to content

Conversation

@Arlodotexe
Copy link
Member

@Arlodotexe Arlodotexe commented May 2, 2022

This PR

  • Splits Project Reference generation into a new DiscoverSamples.ps1 file
  • Fixes launching all-sample app from the dropdown in VS
  • Adds automatic discovery of individual sample projects for launch.json.
  • Closes Codespaces can't find SampleRefs.props #89

Demos:

All-Samples-In-Codespaces-1.mp4
Sample-Discovery-Codespaces-1.mp4

@Arlodotexe Arlodotexe requested a review from michael-hawker May 2, 2022 18:22
"name": "All samples app",
"type": "coreclr",
"request": "launch",
"program": "dotnet",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we want to add this command as a separate job on the CI?

Copy link
Member Author

@Arlodotexe Arlodotexe May 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't include in this PR because it isn't as easy as running a command in the CI.

To do this proper, we'd want to

  1. Investigate if we can validate the JSON as a VSCode config using some command line tool (make sure invalid json isn't generated or committed)
  2. Write a script to copy the command and parameters programmatically by reading the JSON file, then execute it to make sure it does fail.
  3. Repeat step 1 and 2 with the generated "discovered" sample launch configs.

@michael-hawker michael-hawker added bug 🐛 Something isn't working WASM Bugs related to working with WASM/Codespaces build 🔥 sample app 🖼 labels May 3, 2022
@michael-hawker michael-hawker force-pushed the fix/codespace-sample-launching branch from b5a6c40 to add326b Compare May 4, 2022 05:14
@michael-hawker
Copy link
Member

Rebased on top of main to grab the changes from #85

Set-Content -Path $generatedSampleRefsPropsPath -Value $sampleRefsPropsTemplate;
Write-Output "Sample project references generated at $generatedSampleRefsPropsPath";
# Run sample discovery
& ./DiscoverSamples.ps1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work if the script is executed from a different directory? (Don't think that'll be a common occurrence, but curious as I know I had to use a different method in the past to grab the current directory of where the script was located that is being executed.)

Copy link
Member Author

@Arlodotexe Arlodotexe May 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this would work if ran from a different directory, same way relatives paths break in UseUnoWinUI.ps1 and UseTargetFrameworks.ps1 when run from the wrong directory.

@michael-hawker
Copy link
Member

When running the all solution - Looks like the docs are adding more to the path in Codespaces (this worked in VS Code locally last I checked for WASM):

image

The files themselves are in the expected location (same as they are locally). So it looks like it may be the source generator output itself. (Tested a single experiment and that's working properly for the CS/XAML files to display the individual samples, but still fails on the doc with the same incorrect path.)

Added the generated output from the SG to investigate (@mrlacey I had to add the EmitCompilerGeneratedFiles to the Labs.Sample.props not the Labs.Head.props to see then), but that shows the wrong path here:

FilePath = @"/workspaces/Labs-Windows/labs/CanvasLayout/samples/CanvasLayout.Sample/canvaslayout.md"

I suspect this is Windows vs. Linux slashes, will try that quick with the string array for the split and see what happens.

@Arlodotexe
Copy link
Member Author

Arlodotexe commented May 4, 2022

When running the all solution - Looks like the docs are adding more to the path in Codespaces (this worked in VS Code locally last I checked for WASM):

Did a quick test in a Codespace on main (manually running the build command since launch.json is broken there), looks like this issue is an uncaught problem with #85 and not related to this PR.
image

@michael-hawker Is this a prerequisite for merging this, could we do this in another PR? Since this PR is only related to launching in a codespace and sample discovery for the VSCode dropdown, we should be good to go?

@michael-hawker
Copy link
Member

michael-hawker commented May 4, 2022

@Arlodotexe it's a one-line fix, so I'll add it. I did see this trying to run Discover Samples directly:

image

(If I ran the all samples instead first then it ran the generate solution and then discover samples fine...)

After installing the powershell extension I got this:

image

@Arlodotexe
Copy link
Member Author

Arlodotexe commented May 4, 2022

@Arlodotexe it's a one-line fix, so I'll add it. I did see this trying to run Discover Samples directly:

That's so strange, I did this entire PR in a codespace. I'll test again with a fresh container and see if I can repro.

@Arlodotexe Arlodotexe requested a review from michael-hawker May 4, 2022 21:11
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still some known and unknown hiccups with how Codespaces works, but everything here looking good now, thanks!

@michael-hawker michael-hawker added this to the Initial Release milestone May 4, 2022
@michael-hawker michael-hawker merged commit 60927b0 into main May 4, 2022
@delete-merged-branch delete-merged-branch bot deleted the fix/codespace-sample-launching branch May 4, 2022 21:37
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
…space-sample-launching

Codespaces: Fix launching all-sample app, add auto sample discovery
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
…1/gallery-improvements

Gallery improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐛 Something isn't working build 🔥 sample app 🖼 WASM Bugs related to working with WASM/Codespaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codespaces can't find SampleRefs.props

2 participants