Skip to content

Enable Try.NET with the new syntax #3769

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

Merged
merged 18 commits into from
Mar 12, 2020
Merged

Enable Try.NET with the new syntax #3769

merged 18 commits into from
Mar 12, 2020

Conversation

mairaw
Copy link
Contributor

@mairaw mairaw commented Jan 14, 2020

@WilliamAntonRohm FYI - this is a new syntax being shipped for Try .NET
With this, you don't need to modify the samples anymore and we can use whole classes

Internal Review Example

@dotnet-bot dotnet-bot added this to the January 2020 milestone Jan 14, 2020
@mairaw mairaw self-assigned this Jan 14, 2020
@BillWagner
Copy link
Member

Hmmmm. I tried on the review site here and the focus mode isn't working. I don't know if that's a resource issue or not.

@LadyNaggaga
Copy link

@BillWagner I tried it as well see the issue.

Copy link
Contributor

@wadepickett wadepickett left a comment

Choose a reason for hiding this comment

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

@mairaw and @BillWagner I get the same focus issue.
Selecting "run" on the first example results in the UI popping up, but the code does not fill in and you get an endless "busy" green button. I can paste in the code manually after that step, but you don't get an option to run, the run button just has the rotating circle.

Selecting "run" on the second example on the page copies the code over, but we still get the "busy" green button and it is locked up.

@mairaw
Copy link
Contributor Author

mairaw commented Jan 14, 2020

I can't remember if it was doing that before I added the second commit. Let me do a different change on a separate API to test.

@mairaw
Copy link
Contributor Author

mairaw commented Jan 15, 2020

So the new example built and it makes no difference.
https://review.docs.microsoft.com/en-us/dotnet/api/system.math.round?view=netframework-4.8&branch=pr-en-us-3769

The first interactive sample uses the old syntax and runs fine.

The other one uses the new syntax and the issue happens again.

@mairaw
Copy link
Contributor Author

mairaw commented Jan 17, 2020

@rloutlaw here are my findings with my latest tests:

  • Using just the ID number instead of whole snippet name doesn't work. It just loads the entire file.
    Change: 6146276
    Review URL
  • Same happens if I put a bad snippet ID. Expected: ideally I'd get a warning or a missing snippet (current behavior with the other syntax I believe)
    Change: 5f452e2
    Review URL
  • snippet tag with inner snippet tags - inner tags are shown
    Review URL
  • Can we use the # syntax like we used to for source paths? Or is that not supported in the new syntax?

@mairaw mairaw changed the title Enable Try.NET with the new syntax WIP Enable Try.NET with the new syntax Jan 24, 2020
@mairaw
Copy link
Contributor Author

mairaw commented Feb 14, 2020

Changing the interactive type made the sample load and run.
https://review.docs.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?branch=pr-en-us-3769&view=netframework-4.8#code-try-1

However, we still need to clear the snippets comments from the resulting code.

@wadepickett
Copy link
Contributor

wadepickett commented Feb 14, 2020

Works now, hooray! I ran it on Chrome and Edge.
C# and C++ compile, run and exit. (Run not available VB and F#.)

@mairaw
Copy link
Contributor Author

mairaw commented Feb 14, 2020

You scared for a second @wadepickett 😄 There's no C++ code running, It's just missing so it displays C# code as a fallback.

@wadepickett
Copy link
Contributor

@mairaw , yes, sorry, no C++ code running of course!
The language drop down does stick and show C++ even though the page is C#, which it also does on the live site. Is that an issue already filed by chance?

@mairaw
Copy link
Contributor Author

mairaw commented Feb 28, 2020

I think the behavior you're seeing @wadepickett is by design. When you select C++, if there are no C++ samples available, it will show an example from a fallback language. In this case, C#.
Are you seeing anything different?

@mairaw mairaw requested review from eiriktsarpalis, layomia and a team as code owners February 28, 2020 01:00
@BillWagner BillWagner modified the milestones: January 2020, March 2020 Mar 2, 2020
@mairaw mairaw closed this Mar 3, 2020
@mairaw mairaw reopened this Mar 3, 2020
@mairaw
Copy link
Contributor Author

mairaw commented Mar 3, 2020

Checking if bad ID will be caught by build now

@mairaw mairaw closed this Mar 4, 2020
@mairaw mairaw reopened this Mar 4, 2020
@mairaw
Copy link
Contributor Author

mairaw commented Mar 4, 2020

Snippet comments stripping was deployed to production. Testing.

@mairaw
Copy link
Contributor Author

mairaw commented Mar 4, 2020

Status update:

  • Build warnings are still not reported for bad snippet IDs
  • Snippet comments are being stripped but not totally. If you reference the entire file, the most external comments are still rendering

@mairaw mairaw changed the title WIP Enable Try.NET with the new syntax Enable Try.NET with the new syntax Mar 11, 2020
@mairaw
Copy link
Contributor Author

mairaw commented Mar 11, 2020

Status update:

  • Build warnings are still not reported for bad snippet IDs (and now the sample is simply not loaded and is loading a different language sample instead)
  • Snippet comments are being stripped but not totally. If you reference the entire file, the most external comments are still rendering but the workaround is to reference that snippet ID surrounding the snippet, unless you want to update the sample.

This should be good to merge once we confirm that my latest change works for the system.object page

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

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

this LGTM @mairaw

I'll :shipit: now.

@BillWagner BillWagner merged commit 788306c into master Mar 12, 2020
@BillWagner BillWagner deleted the mairaw-patch-1 branch March 12, 2020 16:57
@BillWagner
Copy link
Member

/cc @tdykstra Here are try.net examples using the new syntax. It might help on your project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants