material-extensions / vscode-material-icon-theme

Available on the VSCode Marketplace
https://marketplace.visualstudio.com/items?itemName=PKief.material-icon-theme
MIT License
2.02k stars 622 forks source link

Angular/NestJS Icons within a Nx Monorepo #2233

Open KerickHowlett opened 6 months ago

KerickHowlett commented 6 months ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

  1. Create a Nx Workspace using npx create-nx-workspace@latest --preset=empty
  2. Add Angular Nx Plugin using npx nx add @nx/angular.
    • If command fails, just rerun the command, where it'll most likely work the second time around.
  3. Add Angular Nx Plugin using npx nx add @nx/nest.
    • If command fails, just rerun the command, where it'll most likely work the second time around.
  4. Use Nx to generate both an Angular and NestJS application.
    • It's easiest using Nx's Visual Studio Code or IntelliJ extension, which provides an intuitive UI to generate the application scaffolding for both frameworks.

Expected behavior Have the NestJS icons only appear for the NestJS files and the Angular icons only appear for the Angular files, regardless of the near identical naming convention.

Computer information (please complete the following information):

PKief commented 4 months ago

@lucas-labs as you already know the code structure a bit, do you have any idea of how we could solve the issue that we can always only select one icon pack at once? Initially, icon packs were introduced to avoid conflicts for some file icons. E.g. *.service.ts files can have different icons if you either use Angular or Nest in your project, same applies for other icon packs. But sometimes you can have monorepos where both of the icon packs would be helpful depending on which part of the repo you are working on.

Right now, I only see the possibility of having either a generic icon, that covers both frameworks (as shown in #808) or choosing one of the frameworks as main framework and change the selected icon pack from time to time.

mallowigi commented 4 months ago

Or give the ability to specify a "root directory" that uses the angular pack/nest pack.

For ex you can say "if encountering folder "angular frontend" (configurable per project)" then assume the angular pack is on for any of the file associations inside it

lucas-labs commented 4 months ago

Hi, @PKief, the issue is that when the pack is not active, then the disableIconsByPack filters the icon out of the list of available icons, which is used later to check if the icon is a clone or not.

Since that info isn't available, then the icon definition is being created as if it was a normal icon, which is wrong. Only thing I can think of is modifying the getCustomIcons function to try to find the orignal icon from the unfiltered list from folderIcons.ts/fileIcons.ts and if we found the assigned icon there, then we copy the cloning data to the newly created assigment.

I performed a quick test and yeah, it's working. The only caveat is that it would loop over the entire list of icons (again), adding a little bit of a performance overhead. Although, it would only happen when the user's config is updated, I don't know if I'm happy with that... but I'm kind of a performance freak, so it might be ok haha

lucas-labs commented 4 months ago

Or give the ability to specify a "root directory" that uses the angular pack/nest pack.

For ex you can say "if encountering folder "angular frontend" (configurable per project)" then assume the angular pack is on for any of the file associations inside it

That would also be nice, but unfortunatelly, I'm afraid it's not possible to implement such a feature until vscode supports regex or proper wildcard assignments. Currently, vscode only supports one level of deepness. So, we are able to assign let's say backend/controller.ts but we would end with something like this:

image (I assigned the kubernates icon, because it was the first one I saw in the list)

Although it works for immediate children of backend, it doesn't work for the rest of the tree

PKief commented 4 months ago

Hi, @PKief, the issue is that when the pack is not active, then the disableIconsByPack filters the icon out of the list of available icons, which is used later to check if the icon is a clone or not.

Since that info isn't available, then the icon definition is being created as if it was a normal icon, which is wrong. Only thing I can think of is modifying the getCustomIcons function to try to find the orignal icon from the unfiltered list from folderIcons.ts/fileIcons.ts and if we found the assigned icon there, then we copy the cloning data to the newly created assigment.

I performed a quick test and yeah, it's working. The only caveat is that it would loop over the entire list of icons (again), adding a little bit of a performance overhead. Although, it would only happen when the user's config is updated, I don't know if I'm happy with that... but I'm kind of a performance freak, so it might be ok haha

Yeah, iterating the whole list of icons might not be good regarding the performance. I'm not pretty sure if we should do that as this is rather an edge case than a common use case. As the list of icons will be growing over time, we should be careful about such iterations.

mallowigi commented 4 months ago

Yeah I thought VSCode was using a visitor design pattern but instead it only takes the filename and possibly the parent and assigns a class to it, so while this would work for immediate children, other children would not get affected.

Another idea comes to mind, since settings can be per workspace/project, wouldn't it work if, in the context of a monorepo, the user decides to open each subproject (for example, one project is angular themed and the other is nest themed) and assign a different pack?

lucas-labs commented 4 months ago

Yeah, iterating the whole list of icons might not be good regarding the performance. I'm not pretty sure if we should do that as this is rather an edge case than a common use case. As the list of icons will be growing over time, we should be careful about such iterations.

I'm not a fan either, mainly because there are a few iterations over the lists already happening. E.g.: disableIconsByPack iterates over the lists, then its result is being concatenated with custom icons and then there's another loop over the resulting array.

I'll see if there's an elegant way to take advantage of one of those iterations and reuse them to solve the issue there. But that would probably be next week.

In the meantime, since both nest and angular base icons are not restricted to any IconPack, they are not being filtered out. So, clonining those should do the trick as a workaround (as opposed to using files.associations assigned to let's say nest-controller, which is being filtered out unless the Nest pack is active)