Add support for Type III and Type IV GHE configurations in consult-gh-forge

[lhernanz]

See magit/forge#174 (comment) for more context. But the TLDR is that hosts can have more than /api in the path and that makes the method that retrieves the credentials from the gh command line to fail. The fix could be to remove all the path components from host by using something like:

((string-match "/" host) (string-trim-right host "/.*"))

instead of the current code here:

consult-gh/consult-gh-forge.el

Line 247 in 28f4725

((string-suffix-p "/api" host) (string-trim-right host "/api"))

[armindarvish]

@lhernanz, the solution you are suggesting seems too general to me, and can lead to other issues later. (string-match "/" host) would match anything with /! and (string-trim-right host "/.*") would remove everything after a single /. Can you suggest something more specific?

[lhernanz]

Hi, You are right, but the rationale is the following: in the gh command line utility, the hostname is a real hostname when dealing with accounts. It is only forge that it is overloading this field with a path component to be able to find the api endpoint. Therefore, in order to translate from path to hostname, everything after the / has to go. There might be some corner cases that I am not aware of. Are you aware of any? If you prefer, we can look for all the expected paths and only remove these, but I do not see any scenario where having a path in the hostname would produce any positive outcome when interacting with gh accounts. What do you think? El vie, 27 dic 2024 a las 14:21, Armin Darvish ***@***.***>) escribió:

…

@lhernanz <https://github.com/lhernanz>, the solution you are suggesting seems too general to me, and can lead to other issues later. (string-match "/" host) would match anything with /! and (string-trim-right host "/.*") would remove everything after a single /. Can you suggest something more specific? — Reply to this email directly, view it on GitHub <#182 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFWJ5I46RQRNXHBH73CCGD2HXHHZAVCNFSM6AAAAABUILTVFOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRUGA3DMMRXHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[armindarvish]

If you prefer, we can look for all the expected paths and only remove
these, but I do not see any scenario where having a path in the hostname
would produce any positive outcome when interacting with gh accounts.

I do actually prefer that. As you said, this is only for converting host name from forge names to what gh uses, and therefore we should specifically remove what forge adds rather than using a general pattern becaue I think this would be much easier to maintain over time.