Support scmResourceState in when clauses

[mjcrouch]

• Adds contextValue?: string to the SourceControlResourceState
• Allows when clause in scm/resourceState/context menus to use scmResourceState

Example - I have hidden the "revert" item from the inline and context menu using a context clause like scmResourceState == opened

This PR fixes #86180

[joaomoreno]

It's actually a perf issue if we create menus for each element. So... could we share them between elements if they have the same context key?

[mjcrouch]

I'll have a look at that - my initial implementation was caching the resource menu by resource using a map in ISCMResourceGroup and disposing them with the group (but I realised this was pointless as the menu was ultimately only generated once per resource anyway).

I could make that a map based on contextValue instead so there is one menu per context per group. That way git for example would have exactly the same number of menus created as it does now

[mjcrouch]

@joaomoreno - updated with one menu per context

[mjcrouch]

@joaomoreno now the endgame is done, could we look at merging this if the approach is ok?

(I wasn't 100% sure if the the proposed API was the right place for this)

[mjcrouch]

@joaomoreno any chance of getting this moved on please?

[mjcrouch]

Is there anything blocking this? is it because I didn't write to santa claus this year? I knew I should have put this on my wishlist

[gjsjohnmurray]

@joaomoreno I'd like to add my voice to the request to get this merged. Or is there a reason it can't go in?

[eamodio]

@mjcrouch I haven't looked at this too deeply (so take this with a grain of salt) -- but is there a need for the createResourceMenu/ISCMResourceMenu work? Since the menus are currently already loaded per-group for an individual resource, I would think that just by adding the new contextKeyService.createKey('scmResourceState', resource.contextValue) calls would be enough.

Is that not the case?

[mjcrouch]

@eamodio thanks for reviewing - it's been quite a while since I wrote the code so I'm stretching my memory, but iirc there is currently just one menu instance for each group, and getResourceMenu returned the group menu - ( called from here )

So in order to render different context items for different resource objects, we need to create a menu per contextValue in the group and then return the correct menu according to the contextValue on the object we've clicked / hovered on

[eamodio]

@mjcrouch You might be right, but I would recommend giving it a shot to see if it works properly without it.

[mjcrouch]

@eamodio I gave it a try (I think this may have been what I tried originally) - it does seem to work for the right click context menu, but does not work for the inline icons; they all end up with the same set of icons even if they have different contextValues.

At a total guess I wonder if there is some work done to update the items on right click that maybe is not done on hover for performance reasons

[eamodio]

Hrm, I think it has to do with the connectPrimaryMenuToInlineActionBar calls in scmViewPane.ts -- @joaomoreno will no better as to why that is special.

[joaomoreno]

Assigning for August so I get a good look at this next week. 👍

[mjcrouch]

just happened to notice #104089 - suspect there may pretty sure there will be a conflict when that gets merged :)

[joaomoreno]

Yeah, for sure. No worries, I'll handle the conflicts. 👍

[joaomoreno]

Thanks for the PR, @mjcrouch, it was really polished!👏 I merged it into the latest master and resolved all conflicts. There was a lot of refactorings on menus.ts for perf reasons.

[mjcrouch]

thank you - this will really help to make my menus more usable :)