Initial version supporting multiple workspace roots #48

jvdprng · 2024-09-10T09:36:03Z

This PR adds support for multiple workspace roots. It is currently in draft for testing purposes.

fcasal · 2024-09-20T14:41:00Z

package.json

+                    "command": "weAudit.setupRepositoriesCurrent",
+                    "when": "view == gitConfig",
+                    "group": "navigation@2"
+                },
+                {
+                    "command": "weAudit.setupRepositoriesAll",
+                    "when": "view == gitConfig",
+                    "group": "navigation@3"
+                },


We might be able to only show the setupRepositoriesAll button when there are multiple repos

This is resolved in 88aff0e

fcasal · 2024-09-20T14:48:38Z

src/codeMarker.ts

+        // if (!this.rootPath) { // TODO: Should not happen?
+        //     return;
+        // }


Remove or uncomment

This is resolved in 9ae4bf7

fcasal · 2024-09-20T14:52:03Z

src/codeMarker.ts

+     * Toggle a file as audited.
+     * @param uri the `uri` of the target file.
+     * @param relativePath the relative path of the target file to this workspace root.
+     * @returns A list of `uri`s to decorate and the relevant username. TODO


This is resolved in 9ae4bf7

fcasal · 2024-09-20T14:56:49Z

src/codeMarker.ts

+
+        return [urisToDecorate, relevantUsername];
+    }
+


Most of the following functions are missing return types on their signatures

This is resolved in 9ae4bf7

fcasal · 2024-09-20T14:58:02Z

src/codeMarker.ts

+     * @returns `true` if the file is in the AuditedFiles, `false` if not.
+     */
+    isAudited(path: string) {
+        return this.auditedFiles.find((entry) => entry.path === path);
+    }


This function does not return true or false

This is resolved in 9ae4bf7

fcasal · 2024-09-20T16:19:59Z

src/multiConfigs.ts

+        const disposable = vscode.workspace.onDidChangeWorkspaceFolders(listener);
+        context.subscriptions.push(disposable);
+
+        this.activeConfigs = [];


this.activeConfigs is already set on line 31

This is resolved in 9ae4bf7

fcasal · 2024-09-20T16:21:14Z

src/multiConfigs.ts

+                    // if (parsedPath.name === this.username && this.activeConfigs.length === 0) {
+                    //     this.activeConfigs.push(entry);
+                    // }


I think this should be uncommented

This is resolved in 9ae4bf7

fcasal · 2024-09-20T16:24:05Z

src/panels/gitConfig.html

+    <div class="detailsDiv">
+        <vscode-dropdown position="below" id="workspace-root-list-dropdown"> </vscode-dropdown>
+    </div>


We could hide this if there is only one workspace. We can leave it as future enhancement

Left it for now

fcasal · 2024-09-20T16:27:18Z

src/panels/gitConfigPanel.ts

+                            // TODO error
+                            return;
+                        }
+                        vscode.commands.executeCommand("weAudit.updateGitConfig", rootPath, message.clientURL, message.auditURL, message.commitHash);
+                        return;
+                    case "choose-workspace-root":
+                        rootPath = this.dirToPathMap.get(message.rootDir);
+                        if (rootPath === undefined) {
+                            // TODO error
+                            return;
+                        }
+                        //this.currentRootPath = message.rootPath;


Couple of todos and commented code

This is resolved in 9ae4bf7

fcasal · 2024-09-20T16:32:13Z

src/webview/gitConfigMain.ts

+                rootList.textContent = "";
+                for (let i = 0; i < message.rootDirs.length; i++) {
+                    const option = document.createElement("vscode-option");
+                    //option.value = message.rootDirs[i];


Let's remove it if we don't need it

This is resolved in 9ae4bf7

fcasal · 2024-09-23T11:34:27Z

There is a bug that causes issues to disappear from one workspace. To reproduce:

have one 2-region finding in workspace A, one regular finding in workspace B
hide the workspace B findings
drag and drop one of the 2 regions out to create 2 findings
unhide workspace B -> the workspace B finding will disappear

fcasal · 2024-09-23T11:45:51Z

have two workspaces without findings
create finding on workspace A
press the "Update files" on the WEAUDIT FILES tab
unexpected behavior: the user.weaudit file will appear on both workspaces when it should probably appear on workspace A
bug: the Eye icon is disabled, but the finding is being displayed

…workspace folders.

… containing each other

fcasal · 2024-09-30T06:49:32Z

src/multiConfigs.ts

@@ -11,7 +11,7 @@ let darkEyePath: vscode.Uri;
 export class MultipleSavedFindingsTree implements vscode.TreeDataProvider<ConfigTreeEntry> {
    private configurationEntries: ConfigurationEntry[];
    private rootEntries: WorkspaceRootEntry[];
-    private rootPaths: string[];
+    private rootPathsAndLabels: [string, string][];
    private activeConfigs: ConfigurationEntry[];
    private username: string;


username is not used here anymore

Couldn't find an eslint config that warns about this

fcasal · 2024-09-30T07:31:54Z

src/panels/gitConfigPanel.ts

@@ -14,57 +13,60 @@ export function activateGitConfigWebview(context: vscode.ExtensionContext) {
 class GitConfigProvider implements vscode.WebviewViewProvider {
    public static readonly viewType = "gitConfig";
    private _disposables: vscode.Disposable[] = [];
-    private currentRootPath: string;
+    private currentRootPathAndLabel: [string, string];


Can we rewrite this with an interface so that we don't have to do [0] and [1] through the code?

fcasal · 2024-09-30T07:38:05Z

src/codeMarker.ts

@@ -1217,8 +1523,7 @@ export class CodeMarker implements vscode.TreeDataProvider<TreeEntry> {

    private workspaces: MultiRootManager;
    private username: string;
-    //private currentlySelectedUsernames: string[];
-    private currentlySelectedConfigs: ConfigurationEntry[];
+    //private currentlySelectedConfigs: ConfigurationEntry[];


fcasal · 2024-09-30T07:38:43Z

src/codeMarker.ts

-        vscode.commands.registerCommand("weAudit.manageConfiguration", (config: ConfigurationEntry, select: boolean) => {
-            return this.manageConfiguration(config, select);
-        });
+        // // Returns whether a configuration is currently selected and optionally selects it if not


fcasal · 2024-09-30T07:38:57Z

src/codeMarker.ts

-        }
-        return true;
-    }
+    // /**


…with locations in multiple workspace roots

…cations in different workspace roots

jvdprng added 5 commits September 10, 2024 11:32

Initial version supporting multiple workspace roots

fa182b5

Fix multi-config bug and run prettier

a796474

Run ESLint

a52401a

Fix (partially) audited bug by reading what concat actually does.

4f19eee

Run prettier.

a1cd279

jvdprng marked this pull request as ready for review September 20, 2024 10:42

jvdprng requested review from Vasco-jofra and fcasal as code owners September 20, 2024 10:42

fcasal requested changes Sep 20, 2024

View reviewed changes

jvdprng mentioned this pull request Sep 23, 2024

Request for feature feedback: Support for weaudit files in subdirectories #55

Open

jvdprng added 6 commits September 26, 2024 10:05

Merge main, incorporate configurable username support

87dc2ee

Only show setup multiple repositories button when there are multiple …

88aff0e

…workspace folders.

Process review comments and fix bugs, add support for workspace roots…

9ae4bf7

… containing each other

Add support for colliding workspace root directory names

b1fb817

Better label collision handling, bug fixes

f7659b8

Code comments

d96ffda

fcasal requested changes Sep 30, 2024

View reviewed changes

jvdprng added 2 commits September 30, 2024 11:04

Fix bug with newly created .weaudit files and process review comments

ff4f6c2

Fix bug with newly created .weaudit files and process review comments

0f500bf

fcasal mentioned this pull request Oct 1, 2024

Add support for ignoring files #54

Open

jvdprng added 7 commits October 1, 2024 10:34

Fix editor decoration when file is in multiple workspace roots

a0af8cf

Add support for toggling audited in multiple containing roots

da1a1a9

Fix opening github link

fb49d1d

Add labels to day log, prevent Sarif Explorer from creating findings …

26a7d6c

…with locations in multiple workspace roots

Now really fix the problem of Sarif Explorer creating entries with lo…

d0098e0

…cations in different workspace roots

Basically what the previous commit was supposed to do

ccd702b

Split out entries for Sarif Explorer -> openGitHubIssue as well

de43fb0

fcasal approved these changes Oct 3, 2024

View reviewed changes

jvdprng merged commit 48bb0f8 into main Oct 3, 2024
4 checks passed

jvdprng deleted the jp/multiple-workspaces branch October 3, 2024 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial version supporting multiple workspace roots #48

Initial version supporting multiple workspace roots #48

jvdprng commented Sep 10, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal Sep 20, 2024

jvdprng Sep 26, 2024

fcasal commented Sep 23, 2024

fcasal commented Sep 23, 2024

fcasal Sep 30, 2024

fcasal Sep 30, 2024

fcasal Sep 30, 2024

fcasal Sep 30, 2024

fcasal Sep 30, 2024

fcasal Sep 30, 2024

Initial version supporting multiple workspace roots #48

Initial version supporting multiple workspace roots #48

Conversation

jvdprng commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fcasal commented Sep 23, 2024

fcasal commented Sep 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment