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.

Sign up for GitHub

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

Expand targets names while searching for matching build task #3009

Merged
merged 9 commits into from
Mar 16, 2023
Merged

Expand targets names while searching for matching build task #3009

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b39c24c
fix(CMakeTaskProvider): expand targets names while searching for mat…
piomis Feb 7, 2023
aa13845
feat(cmakeTaskProvider): Pass expansionOptions as argument instead of…
piomis Feb 10, 2023
6b5cc74
Merge branch 'main' into feature/2970_resolve_variables_in_targets_name
piomis Feb 10, 2023
a89030a
expand the options cwd
elahehrashedi Feb 13, 2023
ae9308c
Merge branch 'main' into feature/2970_resolve_variables_in_targets_name
piomis Feb 17, 2023
61bec6f
Merge branch 'main' into feature/2970_resolve_variables_in_targets_name
elahehrashedi Mar 16, 2023
407217f
some final changes
elahehrashedi Mar 16, 2023
b414191
lint
elahehrashedi Mar 16, 2023
d6091f8
fix
elahehrashedi Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Bug Fixes:
- CMake kits fails when parsing exported functions after running environmentSetupScript. [#2676](https://github.com/microsoft/vscode-cmake-tools/issues/2686)
- Implement cmake.parseBuildDiagnostics. [#1932](https://github.com/microsoft/vscode-cmake-tools/issues/1932)
- CMake tools not fully loaded when opening multi-project folders. [#3000](https://github.com/microsoft/vscode-cmake-tools/issues/3000)
- Expand variables in task's targets while searching matching taks. [#2970](https://github.com/microsoft/vscode-cmake-tools/issues/2970) [@piomis](https://github.com/piomis)

## 1.13.45
Bug Fixes:
Expand Down Expand Up @@ -107,7 +108,7 @@ Bug Fixes:
- Revert back to the previous CMake language server extension dependency. [PR #2599](https://github.com/microsoft/vscode-cmake-tools/pull/2599)

Bug Fixes:
- Ninja is used as a default generator. [#2598](https://github.com/microsoft/vscode-cmake-tools/issues/2598)
- Ninja is used as a default generator. [#2598](https://github.com/microsoft/vscode-cmake-tools/issues/2598)

## 1.11.25
Improvements:
Expand Down
25 changes: 19 additions & 6 deletions src/cmakeTaskProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import * as preset from '@cmt/preset';
import { UseCMakePresets } from './config';
import * as telemetry from '@cmt/telemetry';
import * as util from '@cmt/util';
import * as expand from '@cmt/expand';

nls.config({ messageFormat: nls.MessageFormat.bundle, bundleFormat: nls.BundleFormat.standalone })();
const localize: nls.LocalizeFunc = nls.loadMessageBundle();
Expand Down Expand Up @@ -186,18 +187,30 @@ export class CMakeTaskProvider implements vscode.TaskProvider {
return task;
}

public static async findBuildTask(presetName?: string, targets?: string[]): Promise<CMakeTask | undefined> {
public static async findBuildTask(presetName?: string, targets?: string[], expansionOptions?: expand.ExpansionOptions): Promise<CMakeTask | undefined> {
// Fetch all CMake task from `tasks.json` files.
const allTasks: vscode.Task[] = await vscode.tasks.fetchTasks({ type: CMakeTaskProvider.CMakeScriptType });
const tasks: (CMakeTask | undefined)[] = allTasks.map((task: any) => {

const tasks: (CMakeTask | undefined)[] = await Promise.all(allTasks.map(async (task: any) => {
if (!task.definition.label || !task.group || (task.group && task.group.id !== vscode.TaskGroup.Build.id)) {
return undefined;
}

let taskTargets: string[];
if (expansionOptions) {
taskTargets = await expand.expandStrings(task.definition.targets, expansionOptions);
if (task.definition.options?.cwd){
Copy link
Contributor

Choose a reason for hiding this comment

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

I added cwd to be expanded too

task.definition.options.cwd = await expand.expandString(task.definition.options.cwd, expansionOptions);
}
} else {
taskTargets = task.definition.targets;
}

const definition: CMakeTaskDefinition = {
type: task.definition.type,
label: task.definition.label,
command: task.definition.command,
targets: task.definition.targets || targets,
targets: taskTargets || targets,
preset: task.definition.preset,
options: task.definition.options
};
Expand All @@ -208,7 +221,7 @@ export class CMakeTaskProvider implements vscode.TaskProvider {
buildTask.isDefault = true;
}
return buildTask;
});
}));

const buildTasks: CMakeTask[] = tasks.filter((task) => task !== undefined) as CMakeTask[];

Expand Down Expand Up @@ -237,12 +250,12 @@ export class CMakeTaskProvider implements vscode.TaskProvider {
} else {
// Search for the matching default task.
const defaultTask: CMakeTask[] = matchingTargetTasks.filter(task => task.isDefault);
if (defaultTask.length === 1) {
if (defaultTask.length >= 1) {
return defaultTask[0];
} else {
// Search for the matching existing task.
const existingTask: CMakeTask[] = matchingTargetTasks.filter(task => !task.isTemplate);
Copy link
Contributor

@elahehrashedi elahehrashedi Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't see a task.isTemplate in tasks after you fetch tasks using vscode.tasks.fetchTasks. You have to distinguish between the tasks that are fetched from the tasks.json file and the tasks that the task provider provides as a template by setting this value to true/false.

Copy link
Contributor Author

@piomis piomis Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@elahehrashedi
Yes. I mentioned this problem already.
#2936 (comment)
#2780 (comment)
But it was like that in your refactor.

Copy link
Contributor

Choose a reason for hiding this comment

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

INstead of using vscode.tasks.fetchTasks we can write our own function to read the tasks in tasks.json to fetch the existing tasks, and use the function provideTasks to get the template tasks.
Here is a sample:

https://github.com/microsoft/vscode-cpptools/blob/a014fe80bdc03b4a9fa3b877866a11d13034f631/Extension/src/LanguageServer/cppBuildTaskProvider.ts#L218

If you need help implementing this let me know and I will modify your branch.

Copy link
Contributor Author

@piomis piomis Feb 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@elahehrashedi
I'm not sure if manual parsing of Json is good solution (honestly speaking I am not convinced that ad hoc task creation is good solution either).
First of all this will be very fragile in regards to format of Json.
Like I rised in comments - you have added isTemplate property previously but your code never sets it to true. If you point me what place was intended to have set then I will add it.

By the way I found one comment from maintainers of VS code which suggests not to use ad hock tasks (microsoft/vscode#59337 (comment)) maybe an Original proposal with 'target' retrieval during task execution (instead of having target set by default when task is created). Potentially this could also allownto use 'shell' task type.

By the way I have just discovered that ProblemMatchers stopped working. And this is also related to fact that it is not possible to programicaly define non standard problem matcher in Task constructor.

if (existingTask.length === 1) {
if (existingTask.length >= 1) {
return existingTask[0];
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/drivers/cmakeDriver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1763,7 +1763,7 @@ export abstract class CMakeDriver implements vscode.Disposable {
}
const useBuildTask: boolean = this.config.buildTask && isBuildCommand === true;
if (useBuildTask) {
const task: CMakeTask | undefined = await CMakeTaskProvider.findBuildTask(this._buildPreset?.name, targets);
const task: CMakeTask | undefined = await CMakeTaskProvider.findBuildTask(this._buildPreset?.name, targets, this.expansionOptions);
if (task) {
const resolvedTask: CMakeTask | undefined = await CMakeTaskProvider.resolveInternalTask(task);
if (resolvedTask) {
Expand Down