Skip to content
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

Add edit/run for kata examples - issue 591 #1829

Merged

Conversation

ggridin
Copy link
Contributor

@ggridin ggridin commented Aug 10, 2024

Add the edit/run for kata examples.
This PR closes #591

Monaco editor model should have a unique URI (with few exceptions). So, either kata lesson ID or kata exercise ID is provided to the model. The Editor.tsx implementation is replacing "kataExercise" parameter with "kataSection" that contains both lesson.id and exercise.id.
The function parameter "key" is not in use and was removed.

@ggridin
Copy link
Contributor Author

ggridin commented Aug 10, 2024

@swernli Stefan, thank you for the phantom error root cause and monaco editor logic explanation.
I think I have the right fix now.

Testing done:

  1. Successful and failed runs in example editor window.
  2. Hover and autocomplete in example editor window.
  3. Linked code.
  4. Kata exercises and kata samples are working as before.
  5. Code reset.

Example-success-run-and-hover
Example-failed-run-and-autocomplete
Linked-code-successful-run

@ggridin
Copy link
Contributor Author

ggridin commented Aug 10, 2024

Follow-up questions:

  1. Kata exercise and example profile is "unrestricted" only. Should profile selection be deleted from exercises and examples? Sample profile selection remains as is.
  2. Existing problem found and reproduced in https://microsoft.github.io/qsharp/
    If a user selects "Deutsch Algorithm" tutorial, then selects "Deutsch-Jozsa" sample - the sample "Run" button is grayed out.
    Existing-issue-sample-run-is-gray

The fix (most likely) should introduce a unique URI for samples and the linked code.
Should I file a bug for this issue?

@swernli
Copy link
Collaborator

swernli commented Aug 12, 2024

@ggridin Thanks, this is looking good! Regarding the questions,

Kata exercise and example profile is "unrestricted" only. Should profile selection be deleted from exercises and examples? Sample profile selection remains as is.

I think the dropdown for selecting profile on katas, both exercises and examples, is not necessary. They always run in Unrestricted and should remain that way.

The fix (most likely) should introduce a unique URI for samples and the linked code.
Should I file a bug for this issue?

Good catch! Yes, we should follow up on this and fix it, so an issue would be great. I think your suggestion will likely end up being right fix.

@ggridin ggridin marked this pull request as draft August 15, 2024 20:09
@ggridin
Copy link
Contributor Author

ggridin commented Aug 15, 2024

A bug is found: example's syntax error message shows for all examples and exercises. The fix is WIP

@swernli swernli marked this pull request as ready for review August 15, 2024 20:16
@swernli swernli marked this pull request as draft August 15, 2024 20:17
@swernli
Copy link
Collaborator

swernli commented Aug 15, 2024

Sorry for the noise in converting it out of draft, meant to click the button to run the pipeline!

@ggridin
Copy link
Contributor Author

ggridin commented Aug 16, 2024

After today's branch sync, the issue mentioned above is gone (not reproducible).

@ggridin ggridin marked this pull request as ready for review August 16, 2024 00:58
@DmitryVasilevsky DmitryVasilevsky added this pull request to the merge queue Aug 20, 2024
Merged via the queue into microsoft:main with commit df339be Aug 20, 2024
19 checks passed
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.

Kata examples should be editable and runanble
3 participants