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

prompt user to choose parser to parse task output #5877

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

elaihau
Copy link
Contributor

@elaihau elaihau commented Aug 7, 2019

  • before a task runs, prompt the user to choose which parser to use to parse the task output, and write user's choice into tasks.json. Please note: tasks that are already assign problem matchers
  • part of Align UX for tasks with VS Code #4212

How to test

Peek 2019-08-06 22-41

  1. Have a detected task prepared. A configured task with no defined problem matcher also works. If a detected task is involved in the testing, make sure it is successfully loaded as Theia currently has a bug where the detected tasks are not loaded in time some plugins are not loaded in time #5861 (comment)

  2. Run the prepared task, as a user I should see a list of options that advise me what to use to parse the task output.

  • If the task is already assigned a parser, as a user I should not see the list show up.
  1. After the task finishes running,
  • the problems found by the parser should be listed in the problems view
  • the selected problem matcher should be written into the tasks.json as one property of the task.

Review checklist

@elaihau elaihau force-pushed the Liang/ux_add_matcher branch 2 times, most recently from 9d86a05 to 5e35d9a Compare August 7, 2019 03:46
@akosyakov akosyakov added tasks issues related to the task system vscode issues related to VSCode compatibility labels Aug 7, 2019
Copy link
Contributor

@RomanNikitenko RomanNikitenko left a comment

Choose a reason for hiding this comment

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

@elaihau
Hello!
I tested your changes using npm extension and faced with the following case:

  1. Run npm:build task -> drop down menu -> select eslint item
  2. The configuration is moved to tasks.json file and problemMatcher is defined
  3. The parser is assigned to the task, so as a user I should not see the list show up at next running
    It works well.
  4. What user need to do to get again drop down menu and select another item?

I deleted problemMatcher field for the task in tasks.json file and run again the same task - > got drop down menu -> selected another item

As result, I got two configurations with the same label in tasks.json file and in Terminal -> Run Task menu.
Please see the video: https://youtu.be/zpoZ0F0lXok

Can the behavior be improved?
Like here for example: https://youtu.be/SgBuu1-1X5g
WDYT?

thank you!

@elaihau
Copy link
Contributor Author

elaihau commented Aug 8, 2019

@RomanNikitenko thank you for testing, really appreciated !

Yes it is definitely a bug. What I did to "get the drop down menu and select another item" is removing the entire cutomization object from tasks.json, not just the problemMatcher property. So, i never tried what you caught :)

Fixing it is more difficult than I thought, as we don't have the info of where a taskDefinition comes from.

Worse case i might need to change some interfaces. I will see what is the lowest-impact solution to fix it.

@RomanNikitenko
Copy link
Contributor

RomanNikitenko commented Aug 8, 2019

What I did to "get the drop down menu and select another item" is removing the entire cutomization object from tasks.json, not just the problemMatcher property.

I noticed that "problemMatcher": [] was added to config when I selected Never scan the task output, so I thought that I need to remove this field to get the drop down menu again :-)
But you are right - maybe the more obvious case is removing the entire customization object...

@elaihau
Copy link
Contributor Author

elaihau commented Aug 11, 2019

For the information of reviews of this pull reuqest: #5915 is created to fix the issue where task definitions are not loaded before the initialization of tasks. It is a blocker of testing and merging this pull request.

@elaihau
Copy link
Contributor Author

elaihau commented Sep 5, 2019

@vince-fugnitto @RomanNikitenko @azatsarynnyy
i did a rebase. this pull request is review-ready

@azatsarynnyy azatsarynnyy requested a review from a team September 5, 2019 14:34
@elaihau elaihau force-pushed the Liang/ux_add_matcher branch 2 times, most recently from eb39df7 to 7c19815 Compare September 6, 2019 14:02
- before a task runs, prompt the user to choose which parser to use to
parse the task output, and write user's choice into `tasks.json`
- part of #4212

Signed-off-by: Liang Huang <liang.huang@ericsson.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

It looks really good, nice work @elaihau !
I tested the following:

Observation Status
ProblemMatchers are sorted alphabetically 👍
When prompted, choosing continue... will not write to the tasks.json 👍
When prompted, choosing never... will write [] to skip parsing 👍
When prompted, choosing a parser will write the parser successfully 👍
Writing the parser works correct in any location in the tasks.json 👍
Writing the parser works correctly event with a trailing comma 👍
Manually removing a ProblemMatcher successfully resets and the user must be prompted again if he re-runs the task 👍
If an appropriate ProblemMatcher is chosen for a given task, the markers are reflected in the problems-view 👍
If a ProblemMatcher exists for a given task, no prompt is displayed and the ProblemMatcher is used successfully 👍
Run Last Task still works correctly, the task is re-run with its defined ProblemMatcher if available 👍

The code also looks fine to me, I don't have any objections :)

@elaihau
Copy link
Contributor Author

elaihau commented Sep 6, 2019

Thank you Vincent ! Your review covered so many things I didn't try out.

@vince-fugnitto
Copy link
Member

I opened a follow-up issue to enhance the functionality by supporting adding the ProblemMatchers to the task schema so users can benefit from auto-completion and assistance #6134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tasks issues related to the task system vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants