SanderRonde / VSCode-Gerrit

Gerrit plugin for VSCode
https://marketplace.visualstudio.com/items?itemName=SanderRonde.vscode--gerrit
MIT License
29 stars 17 forks source link

"Gerrit: found multiple gerrit roots..." randomly popping up despite valid gerrit repo #66

Closed Mattadore closed 5 months ago

Mattadore commented 6 months ago

Hi, I keep getting this message despite having a valid repo in the settings, and the correct repo does not show in the dropdown, so it's not getting added as a "valid gerrit repo". It doesn't seem to be related to the repo commits not getting parsed, because it will randomly work or not work if I reload, without changing which commit I have open, which makes me feel like it is maybe some networking race? I'm really not sure, I just know it's not consistent, it works sometimes and breaks other times. Any idea?

SanderRonde commented 6 months ago

What this check does is that it goes through each open workspace root and looks whether in the last 50 commits you've got any gerrit commit. That should be really stable since it just uses VSCode's git API to look back into the history.

Do you indeed have a multi-root project? And are any of them also gerrit commits. Could it be that there's a gerrit commit in their history? And could it be that in the gerrit project you mentioned there is not a gerrit commit in the last 50 commits?

For context: I've classified a gerrit commit as a commit containing a change ID, defined as matching the regex: /Change-Id: (([a-zA-Z0-9])?([a-z0-9]{40}))/

Mattadore commented 6 months ago

Nope, single-root and there is 1000% a gerrit commit in the last 50. That said, there are multiple git repos under said root. I'm able to consistently reproduce it by not having any files from the repo I'm trying to use open when I reload VSCode, and then it works if I have a file from said repo open when I reload. Maybe something with VSCode's git integration itself, since it seems like this extension depends on that? Though perhaps having multiple gerrit repos open in the same workspace is not a common use case, especially since this extension only supports running one at a time....? That said having it flat-out disable itself if I don't have a file from the right repo open when I reload is a bit annoying.

SanderRonde commented 6 months ago

I'm not really sure what the best solution is here, but I do have some ideas. VSCode does have an onDidOpenRepository hook so I could not activate the extension while there is no gerrit-root open and activate it when there is. There also is an openRepository function. So I think the best course of action might be:

What do you think of this idea? Something that might still be annoying is that if you have a workspace with multiple git roots with only 1 of them being a gerrit root, the extension won't activate until you open a file within the gerrit git root. Although I guess that's to be expected and there's not really any way around that. It's probably not as much of an issue.

Mattadore commented 5 months ago

Yeah that sounds super workable, and is basically what I was imagining. I will say it's still possible for the gerrit repo you want to not show on the picker, but you could still specify it manually in the settings. Maybe also have a hook to restart the gerrit support if they manually change where it points in settings, if that's feasible? I can imagine a user saying "oh none of these are mine" and having the extension get disabled, or picking a random one, then changing it in the settings after.

Re: "Something that might still be annoying is that if you have a workspace with multiple git roots with only 1 of them being a gerrit root, the extension won't activate until you open a file within the gerrit git root" sure, but it will only happen the first time they use it and after that it gets saved (I think, I'm not sure if the gerrit.gitRepo gets set if there is only one gerrit repo). I don't see any other realistic option anyway, as not using the vscode git support and iterating through all the available gerrit repos is too expensive since some projects (Android) are massive and have hundreds of them.

For my use case I just pick the most common one I use, or change depending on what patch I'm reviewing, and it's fine. Support for multiple would be great but that's way outside the scope of this issue.

SanderRonde commented 5 months ago

Maybe also have a hook to restart the gerrit support if they manually change where it points in settings, if that's feasible

Good point. While I think restarting it by itself is not easy to do, popping up a "hey please reload your VSCode" message might be a big help already.

sure, but it will only happen the first time they use it and after that it gets saved

In that case it won't get saved actually. Reason being that it would need to be saved somewhere, and that somewhere has to be user settings, in this case workspace settings. Having an extension write to your .vscode/settings.json by default when you launch it feels kind of annoying to me, so I opted not to go that route. I think in practice the multi-root with only one being a gerrit repo is maybe not too common of a setup anyway so I think it's probably fine.

Will implement this then :)

SanderRonde commented 5 months ago

I've created a version with the improved picker, can you give it a try?

vscode--gerrit-1.2.34.zip

Mattadore commented 5 months ago

Yep, totally fixes the issue, as far as I can tell. I tested same editor state on both patches and it only worked on the updated one.

SanderRonde commented 5 months ago

Awesome, I'll merge that then