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

fix for "Cannot Create keybinding to restart a specific Task" #36474

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@vemoo

vemoo commented Oct 18, 2017

Issue: #35910.

Also fixes a couple of related issues in src/vs/workbench/parts/tasks/node/processTaskSystem.ts task termination.

@msftclas

This comment has been minimized.

msftclas commented Oct 18, 2017

CLA assistant check
All CLA requirements met.

@dbaeumer dbaeumer added this to the October 2017 milestone Oct 19, 2017

@vemoo

This comment has been minimized.

vemoo commented Nov 12, 2017

I fixed some typos in some translation keys in this commit becase at the time I could't find any translations in the repository. But now I see there are translations, should I revert that change? or update the keys in the translations also?

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Nov 13, 2017

There is no need to revert nor to update the translation. This happens automatically when we pull the next time from our translation database.

@dbaeumer dbaeumer modified the milestones: October 2017, November 2017 Nov 13, 2017

@alexandrudima alexandrudima modified the milestones: November 2017, December 2017 Dec 8, 2017

@vemoo

This comment has been minimized.

vemoo commented Dec 17, 2017

After using vscode for a while I realised this pull request introduces some changes when stopping and restarting tasks.

Right now when terminating or restarting a task, the following happens:

  • in the old task system (0.1.0) the task is terminated or restarted directly
  • in the new task system (2.0.0) even if there's only one task running a quick pick is always show

In this pull request I unified the code so both stop or terminate the task without asking the user if there's only one running.
What's the intended behaviour?

Also, if an argument is passed but no task is found, should there be some kind of notification for the user?

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Dec 18, 2017

@vemoo

In tasks 2.0 we always prompt on terminate even if only one task is running. Since task 2.0.0 can run multiple task in parallel we need a dialog for n >= 2 and to make things consistent we always show the dialog. However if a keyboard short cut is bound to terminate a specific task no picker should be shown.

If an argument is passed but the task is not found I would show a picker with the currently running tasks. This is consistent with a short cut to executing a task.

@vemoo

This comment has been minimized.

vemoo commented Feb 10, 2018

Is there anything else I can do to help this pull request get merged?

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Feb 19, 2018

@vemoo actually I was not sure if your PR addresses these two issues:

However if a keyboard short cut is bound to terminate a specific task no picker should be shown.

If an argument is passed but the task is not found I would show a picker with the currently running tasks. This is consistent with a short cut to executing a task.

@vemoo

This comment has been minimized.

vemoo commented Feb 19, 2018

Yes, with this PR, when terminating a task with an argument

  • If the task is found: it's terminated without showing the picker
  • If the task is not found: the currently running tasks are show in the picker

@bpasero bpasero modified the milestones: February 2018, March 2018 Mar 8, 2018

@vemoo

This comment has been minimized.

vemoo commented Mar 11, 2018

I've rebased onto current upstream master, and fixed the conflict.
Now the changes in this PR also use the "notificationService" instead of the "messageService".

@bpasero bpasero modified the milestones: March 2018, April 2018 Apr 6, 2018

@bpasero bpasero modified the milestones: April 2018, May 2018 May 8, 2018

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented May 30, 2018

The fix for the arg is OK, however there is #47853 which adds a more generic way to skip zero task prompts. Will look into merging the two PRs.

@dbaeumer dbaeumer modified the milestones: May 2018, June 2018 May 30, 2018

@dbaeumer

This comment has been minimized.

Member

dbaeumer commented Jun 7, 2018

A lot has changed here especially the fact to be able to reference a task via a task identifier. So I merged the PR by hand and modified it to:

  • use the picker directly when no arg is provided. This ensures that we have proper progress
  • use task identifier
  • only available for task 2.0.0

@dbaeumer dbaeumer closed this Jun 7, 2018

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