Repository: microsoft/inshellisense
Pull request #207 (2024-03-12 07:57:39)

fix: for filepaths/folders templates, the icon type of the suggestion…
For filepaths / folder templates, the icon type of the suggestion needs to be specified to avoid going to the default icon.

thanks for the review.
@cpendery

<img width="789" alt="image" src="https://github.com/microsoft/inshellisense/assets/9245110/14b7235a-6528-4966-bb04-d85feb5af1cc">


alanhg: > Can you add a test that covers this case, something like `ls sr` and just take the first item so we know it has a folder icon and then do something like `ls READ` to capture that it has the file icon?
> 
> Trying to have all PRs adding tests for the behavior they fix moving forward
Thank you very much for the suggestion, I've added the corresponding test case.



![image](https://github.com/microsoft/inshellisense/assets/9245110/11dc22bf-4527-4476-9582-3788f2f91421)


![image](https://github.com/microsoft/inshellisense/assets/9245110/fb2a7c5b-0b0d-46dc-ace4-0c244b53adb9)

I verified that it was already OK. But I found a problem with the other test cases.

Do you want to fix it immediately or skip it first?

![image](https://github.com/microsoft/inshellisense/assets/9245110/087a89a2-f3ab-45fc-86a4-fc49f20a74b8)


cpendery: > > Can you add a test that covers this case, something like `ls sr` and just take the first item so we know it has a folder icon and then do something like `ls READ` to capture that it has the file icon?
> > Trying to have all PRs adding tests for the behavior they fix moving forward
> 
> @cpendery
> 
> Thank you very much for the suggestion, I've added the corresponding test case.
> 
> ![image](https://private-user-images.githubusercontent.com/9245110/313444851-11dc22bf-4527-4476-9582-3788f2f91421.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTA3Nzc3MzgsIm5iZiI6MTcxMDc3NzQzOCwicGF0aCI6Ii85MjQ1MTEwLzMxMzQ0NDg1MS0xMWRjMjJiZi00NTI3LTQ0NzYtOTU4Mi0zNzg4ZjJmOTE0MjEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMxOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMThUMTU1NzE4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NmE1NmJjMTY5YTExZmRhODQyNThkNTZhMDkzNzc4NDk5Yjc5MTdjYTkyODc4Y2UyZTA4NGI2NTJmZmI0NjNjMSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.aLUyGY35P4MuI3hjpOTTIzk9YC4PTkBemB7tTyxMlNU)
> 
> ![image](https://private-user-images.githubusercontent.com/9245110/313444859-fb2a7c5b-0b0d-46dc-ace4-0c244b53adb9.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTA3Nzc3MzgsIm5iZiI6MTcxMDc3NzQzOCwicGF0aCI6Ii85MjQ1MTEwLzMxMzQ0NDg1OS1mYjJhN2M1Yi0wYjBkLTQ2ZGMtYWNlNC0wYzI0NGI1M2FkYjkucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMxOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMThUMTU1NzE4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MTRjMzdhYWRkMmIzODZlZTYxOGMyODE3MGZiYzk1NWVjNjY4M2E0MzJkMmVlNmE5M2QyNDJmMjE3YTc3MzI5ZCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.Zagy6SWKI-ln8Jz6QSoAj1Onu93I35ClOX9ygIAn-2g)
> 
> I verified that it was already OK. But I found a problem with the other test cases.
> 
> Do you want to fix it immediately or skip it first?
> 
> ![image](https://private-user-images.githubusercontent.com/9245110/313445007-087a89a2-f3ab-45fc-86a4-fc49f20a74b8.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTA3Nzc3MzgsIm5iZiI6MTcxMDc3NzQzOCwicGF0aCI6Ii85MjQ1MTEwLzMxMzQ0NTAwNy0wODdhODlhMi1mM2FiLTQ1ZmMtODZhNC1mYzQ5ZjIwYTc0YjgucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMxOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMThUMTU1NzE4WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YmRmNjc0MDVjYTA2MTgwNzk5ZTkxZmVjMzgyNjg3NjhhNDAzMzdjN2FlZTk2ZTU3ODQwMWU3NmNiOWQxNWU5MiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.6KALiWX9-q-1wUSr7ku_71a_73ZimvLJSUN7oZaGxI8)

If it continues to be an issue, feel free to skip it. I'll address it later
cpendery: @alanhg Looks like you'll need to update the e2e tests too since they also run into those icon issues
alanhg: 
I realized that e2e tests don't check icons or anything like that, this PR just changed the icon of the question, the e2e interaction test itself doesn't need to change, right? Or do you want to add the corresponding test case, but we have already added the check in the unit test.



Also, I found a lot of failures in e2e tests.


![image](https://github.com/microsoft/inshellisense/assets/9245110/1260174e-0869-456d-aef4-20dccfc8f89a)



cpendery: > @cpendery Hi. I realized that e2e tests don't check icons or anything like that, this PR just changed the icon of the question, the e2e interaction test itself doesn't need to change, right? Or do you want to add the corresponding test case, but we have already added the check in the unit test.
> 
> Also, I found a lot of failures in e2e tests.
> 
> ![image](https://private-user-images.githubusercontent.com/9245110/315418546-1260174e-0869-456d-aef4-20dccfc8f89a.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTExMjQxOTksIm5iZiI6MTcxMTEyMzg5OSwicGF0aCI6Ii85MjQ1MTEwLzMxNTQxODU0Ni0xMjYwMTc0ZS0wODY5LTQ1NmQtYWVmNC0yMGRjY2ZjOGY4OWEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMyMiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMjJUMTYxMTM5WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YzI3M2Q4MDAxYzExM2M5YTJmNmQ0MWY4MjQyYmI2ZGY1Njc5ZGFjMGRjNWIyZmNmMTg0ZTU2MGRiNDkwM2FiYiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.paQpd1eupFC85AYqb5vKZFgodtuqLXAws20pqTx_xNk)

The test `generator results lead suggestions` in `src\tests\ui\autocomplete.test.ts` does use the icon of `ls` which needs updated. That's the only test that's failing in the jobs. The e2e framework is definitely still a work in progress [tui-test](https://github.com/microsoft/tui-test). If you could open any issues there, that would be great. 
alanhg: > > @cpendery Hi. I realized that e2e tests don't check icons or anything like that, this PR just changed the icon of the question, the e2e interaction test itself doesn't need to change, right? Or do you want to add the corresponding test case, but we have already added the check in the unit test.
> > Also, I found a lot of failures in e2e tests.
> > ![image](https://private-user-images.githubusercontent.com/9245110/315418546-1260174e-0869-456d-aef4-20dccfc8f89a.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTExMjQxOTksIm5iZiI6MTcxMTEyMzg5OSwicGF0aCI6Ii85MjQ1MTEwLzMxNTQxODU0Ni0xMjYwMTc0ZS0wODY5LTQ1NmQtYWVmNC0yMGRjY2ZjOGY4OWEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMyMiUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMjJUMTYxMTM5WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YzI3M2Q4MDAxYzExM2M5YTJmNmQ0MWY4MjQyYmI2ZGY1Njc5ZGFjMGRjNWIyZmNmMTg0ZTU2MGRiNDkwM2FiYiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.paQpd1eupFC85AYqb5vKZFgodtuqLXAws20pqTx_xNk)
> 
> The test `generator results lead suggestions` in `src\tests\ui\autocomplete.test.ts` does use the icon of `ls` which needs updated. That's the only test that's failing in the jobs. The e2e framework is definitely still a work in progress [tui-test](https://github.com/microsoft/tui-test). If you could open any issues there, that would be great.


I have updated the test, thank you.

<img width="1443" alt="image" src="https://github.com/microsoft/inshellisense/assets/9245110/d48b2263-c563-46c2-b101-33d299009c1f">





Patch:
diff --git a/src/runtime/template.ts b/src/runtime/template.ts
index 24ca78c..894e751 100644
--- a/src/runtime/template.ts
+++ b/src/runtime/template.ts
@@ -6,12 +6,21 @@ import log from "../utils/log.js";
 
 const filepathsTemplate = async (cwd: string): Promise<Fig.TemplateSuggestion[]> => {
   const files = await fsAsync.readdir(cwd, { withFileTypes: true });
-  return files.filter((f) => f.isFile() || f.isDirectory()).map((f) => ({ name: f.name, priority: 55, context: { templateType: "filepaths" } }));
+  return files
+    .filter((f) => f.isFile() || f.isDirectory())
+    .map((f) => ({ name: f.name, priority: 55, context: { templateType: "filepaths" }, type: f.isDirectory() ? "folder" : "file" }));
 };
 
 const foldersTemplate = async (cwd: string): Promise<Fig.TemplateSuggestion[]> => {
   const files = await fsAsync.readdir(cwd, { withFileTypes: true });
-  return files.filter((f) => f.isDirectory()).map((f) => ({ name: f.name, priority: 55, context: { templateType: "folders" } }));
+  return files
+    .filter((f) => f.isDirectory())
+    .map((f) => ({
+      name: f.name,
+      priority: 55,
+      context: { templateType: "folders" },
+      type: "folder",
+    }));
 };
 
 // TODO: implement history template


Test patch:
diff --git a/src/tests/runtime/__snapshots__/runtime.test.ts.snap b/src/tests/runtime/__snapshots__/runtime.test.ts.snap
index 3bccf0e..a5ef57f 100644
--- a/src/tests/runtime/__snapshots__/runtime.test.ts.snap
+++ b/src/tests/runtime/__snapshots__/runtime.test.ts.snap
@@ -505,7 +505,7 @@ exports[`parseCommand optionsSuggestedAfterVariadicArg 1`] = `
         "package-lock.json",
       ],
       "description": undefined,
-      "icon": "📀",
+      "icon": "📄",
       "insertValue": undefined,
       "name": "package-lock.json",
       "priority": 55,
@@ -572,6 +572,42 @@ exports[`parseCommand partialPrefixFilter 1`] = `
 }
 `;
 
+exports[`parseCommand provideFileSuggestion 1`] = `
+{
+  "charactersToDrop": 4,
+  "suggestions": [
+    {
+      "allNames": [
+        "README.md",
+      ],
+      "description": undefined,
+      "icon": "📄",
+      "insertValue": undefined,
+      "name": "README.md",
+      "priority": 55,
+    },
+  ],
+}
+`;
+
+exports[`parseCommand provideFolderSuggestion 1`] = `
+{
+  "charactersToDrop": 2,
+  "suggestions": [
+    {
+      "allNames": [
+        "src",
+      ],
+      "description": undefined,
+      "icon": "📁",
+      "insertValue": undefined,
+      "name": "src",
+      "priority": 55,
+    },
+  ],
+}
+`;
+
 exports[`parseCommand providedArgDescription 1`] = `
 {
   "argumentDescription": "actor",
diff --git a/src/tests/runtime/runtime.test.ts b/src/tests/runtime/runtime.test.ts
index 83e9773..c4032d1 100644
--- a/src/tests/runtime/runtime.test.ts
+++ b/src/tests/runtime/runtime.test.ts
@@ -13,6 +13,8 @@ const testData = [
   { name: "exclusiveOnOption", command: "ag --affinity --no" },
   { name: "providedSuggestion", command: "bw completion --shell " },
   { name: "fullyTypedSuggestion", command: "ls -W" },
+  { name: "provideFolderSuggestion", command: "ls sr" },
+  { name: "provideFileSuggestion", command: "ls READ" },
   { name: "optionsSuggestedAfterVariadicArg", command: "ls item -l", maxSuggestions: 3 },
   { name: "noOptionsSuggestedDuringVariadicArg", command: "ls -W ite" },
   { name: "providedArgDescription", command: "act completion bash -a " },
diff --git a/src/tests/ui/autocomplete.test.ts b/src/tests/ui/autocomplete.test.ts
index 1d2cb29..f02093a 100644
--- a/src/tests/ui/autocomplete.test.ts
+++ b/src/tests/ui/autocomplete.test.ts
@@ -68,7 +68,7 @@ shells.map((activeShell) => {
       await expect(terminal.getByText(">  ")).toBeVisible();
       terminal.write("ls ");
 
-      await expect(terminal.getByText("📀", { strict: false })).toBeVisible();
+      await expect(terminal.getByText("📄", { strict: false })).toBeVisible();
     });
 
     test("tab completion", async ({ terminal }) => {