address reviews

Signed-off-by: kim5566 <28945404+kim5566@users.noreply.github.com>

.changeset/mean-pumpkins-search.md

@@ -2,11 +2,4 @@
 '@backstage/plugin-todo-backend': patch
 ---
 
-Add support to exclude certain folders in `todo` plugin.
-
-To add excluded folders, edit `app-config.yaml` as follow:
-
-```
-todo:
-  excludeFolders: ['vendor/']
-```
+Add support to exclude vendor folder in `todo` plugin.

plugins/todo-backend/api-report.md

@@ -131,7 +131,6 @@ export type TodoScmReaderOptions = {
   reader: UrlReader;
   integrations: ScmIntegrations;
   parser?: TodoParser;
-  excludeFolders?: string[];
 };
 
 // @public (undocumented)

plugins/todo-backend/src/lib/TodoReader/TodoScmReader.test.ts

@@ -192,12 +192,10 @@ describe('TodoScmReader', () => {
 
   it('should filter out exclude folders', async () => {
     const reader = mockReader();
-    const excludeFolders = ['vendor/'];
     const todoReader = new TodoScmReader({
       logger: getVoidLogger(),
       reader,
       integrations: ScmIntegrations.fromConfig(new ConfigReader({})),
-      excludeFolders: excludeFolders,
     });
     reader.readTree.mockResolvedValueOnce({
       files: async () => [
@@ -205,6 +203,10 @@ describe('TodoScmReader', () => {
           content: async () => Buffer.from('// TODO: my-todo', 'utf8'),
           path: 'vendor/another-file.go',
         },
+        {
+          content: async () => Buffer.from('// TODO: my-todo', 'utf8'),
+          path: 'test/vendor/another-file.go',
+        },
         {
           content: async () => Buffer.from('// TODO: my-todo', 'utf8'),
           path: 'my-folder/my-file.js',
@@ -225,6 +227,14 @@ describe('TodoScmReader', () => {
           viewUrl:
             'https://github.com/backstage/backstage/vendor/another-file.go#L1',
         },
+        {
+          text: 'my-todo',
+          tag: 'TODO',
+          lineNumber: 1,
+          repoFilePath: 'test/vendor/another-file.go',
+          viewUrl:
+            'https://github.com/backstage/backstage/test/vendor/another-file.go#L1',
+        },
         {
           text: 'my-todo',
           tag: 'TODO',
@@ -247,6 +257,7 @@ describe('TodoScmReader', () => {
     const filterFunc = reader.readTree.mock.calls[0][1]!.filter!;
     expect(filterFunc('another-file.go')).toBe(true);
     expect(filterFunc('vendor/another-file.go')).toBe(false);
+    expect(filterFunc('test/vendor/another-file.go')).toBe(false);
     expect(filterFunc('my-file.js')).toBe(true);
     expect(filterFunc('my-folder/my-file.js')).toBe(true);
   });

plugins/todo-backend/src/lib/TodoReader/TodoScmReader.ts

@@ -41,13 +41,14 @@ const excludedExtensions = [
 ];
 const MAX_FILE_SIZE = 200000;
 
+const excludeFolders = ['vendor'];
+
 /** @public */
 export type TodoScmReaderOptions = {
   logger: Logger;
   reader: UrlReader;
   integrations: ScmIntegrations;
   parser?: TodoParser;
-  excludeFolders?: string[];
 };
 
 type CacheItem = {
@@ -61,7 +62,6 @@ export class TodoScmReader implements TodoReader {
   private readonly reader: UrlReader;
   private readonly parser: TodoParser;
   private readonly integrations: ScmIntegrations;
-  private readonly excludeFolders: string[];
 
   private readonly cache = new Map<string, CacheItem>();
   private readonly inFlightReads = new Map<string, Promise<CacheItem>>();
@@ -70,11 +70,8 @@ export class TodoScmReader implements TodoReader {
     config: Config,
     options: Omit<TodoScmReaderOptions, 'integrations'>,
   ) {
-    const excludeFolders: string[] =
-      config.getOptionalStringArray('todo.excludeFolders') ?? [];
     return new TodoScmReader({
       ...options,
-      excludeFolders,
       integrations: ScmIntegrations.fromConfig(config),
     });
   }
@@ -84,7 +81,6 @@ export class TodoScmReader implements TodoReader {
     this.reader = options.reader;
     this.parser = options.parser ?? createTodoParser();
     this.integrations = options.integrations;
-    this.excludeFolders = options.excludeFolders ?? [];
   }
 
   async readTodos(options: ReadTodosOptions): Promise<ReadTodosResult> {
@@ -95,12 +91,7 @@ export class TodoScmReader implements TodoReader {
     }
 
     const cacheItem = this.cache.get(url);
-    const excludeFolders = this.excludeFolders;
-    const newRead = this.doReadTodos(
-      { url },
-      excludeFolders,
-      cacheItem?.etag,
-    ).catch(error => {
+    const newRead = this.doReadTodos({ url }, cacheItem?.etag).catch(error => {
       if (cacheItem && error.name === 'NotModifiedError') {
         return cacheItem;
       }
@@ -119,10 +110,13 @@ export class TodoScmReader implements TodoReader {
 
   private async doReadTodos(
     options: ReadTodosOptions,
-    excludeFolders?: string[],
     etag?: string,
   ): Promise<CacheItem> {
     const { url } = options;
+    const filePathFilter = (filePath: string): boolean => {
+      const splitPath = filePath.split('/');
+      return !excludeFolders.some(r => splitPath.includes(r));
+    };
     const tree = await this.reader.readTree(url, {
       etag,
       filter(filePath, info) {
@@ -130,12 +124,11 @@ export class TodoScmReader implements TodoReader {
         if (info && info.size > MAX_FILE_SIZE) {
           return false;
         }
-        const excFolders = excludeFolders ?? [];
         return (
           !filePath.startsWith('.') &&
           !filePath.includes('/.') &&
           !excludedExtensions.includes(extname) &&
-          !excFolders.some(exclude => filePath.startsWith(exclude))
+          filePathFilter(filePath)
         );
       },
     });