Support bazel tasks from tasks.json
#346
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vogelsgesang
requested review from
coeuvre,
jfirebaugh and
cameron-martin
as code owners
vogelsgesang
force-pushed
the
avogelsgesang-bazel-tasks
branch
from
b934692 to
77c922a
Compare
User-facing changes ------------------- This commit fixes two user-facing bugs: * The "Run Task" command provides a list of recently used tasks. This list also contains the build/test tasks started from the "Bazel Build Targets" tree view. However, trying to rerun any of those tasks resulted in an error message. * The extension already contributed a `taskDefinition` for bazel tasks. However, trying to actually define such a task in the `tasks.json` resulted in the same error message. In both cases, the user was presented with the error message > There is no task provider registered for tasks of type "bazel" This commit fixes the issue, such that rerunning tasks from the history and configuring tasks through `tasks.json` now works properly. Furthermore, the `taskDefinition` inside the `package.json` was improved: * It now lists the available `command` options, such that we have auto-completion within the `tasks.json` editor. * It properly constraints the `targets` entries to be string-typed. * It exposes the `options` argument which allows to specify additional command line parameters. Code changes ------------ Fixing the issue mainly required registering a `TaskProvider` for `bazel` tasks. The `TaskProvider` currently does not auto-detect the available tasks, but only implements `resolveTask`. The notifications in `onTaskProcessEnd` had to be refactored: So far, the `BazelTaskInfo` was attached right during task creation. However, this didn't work for the new `resolveTask`. `resolveTask` apparently isn't allowed to change a task's definition. To make notifications work, the `BazelTaskInfo` is now attached in the `onTaskStart` callback. Many of the fields of the `BazelTaskInfo` were unused. This commit removes `commandOptions`, `processId`, `exitCode` all of which were stored but never read. Doing so, it turned out that the `onTaskProcessStart` was no longer necessary. Lastly, the callbacks used for task notifications were moved from `extension.ts` to `bazel_task_info.ts` and activation of all task-related functionality was centralized in the `activateTaskProvider` function
vogelsgesang
force-pushed
the
avogelsgesang-bazel-tasks
branch
from
77c922a to
7397c0c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
User-facing changes
This commit fixes two user-facing bugs:
taskDefinitionfor bazel tasks. However, trying to actually define such a task in thetasks.jsonresulted in the same error message.In both cases, the user was presented with the error message
This commit fixes the issue, such that rerunning tasks from the history and configuring tasks through
tasks.jsonnow works properly.Furthermore, the
taskDefinitioninside thepackage.jsonwas improved:commandoptions, such that we have auto-completion within thetasks.jsoneditor.targetsentries to be string-typed.optionsargument which allows to specify additional command line parameters.Code changes
Fixing the issue mainly required registering a
TaskProviderforbazeltasks. TheTaskProvidercurrently does not auto-detect the available tasks, but only implementsresolveTask.The notifications in
onTaskProcessEndhad to be refactored: So far, theBazelTaskInfowas attached right during task creation. However, this didn't work for the newresolveTask.resolveTaskapparently isn't allowed to change a task's definition. To make notifications work, theBazelTaskInfois now attached in theonTaskStartcallback.Many of the fields of the
BazelTaskInfowere unused. This commit removescommandOptions,processId,exitCodeall of which were stored but never read. Doing so, it turned out that theonTaskProcessStartwas no longer necessary.Lastly, the callbacks used for task notifications were moved from
extension.tstobazel_task_info.tsand activation of all task-related functionality was centralized in theactivateTaskProviderfunctionBEGIN_COMMIT_OVERRIDE
feat: Support bazel tasks from tasks.json (#346)
END_COMMIT_OVERRIDE