gitutil: strip SSH port from submodule URL when rendering for HTML link

[TheDarkUndoing]

Describe the pull request

This is the bug fix for #4941

Link to the issue: fixes #4941 (comment)

Checklist

• I agree to follow the Code of Conduct by submitting this pull request.
• I have read and acknowledge the Contributing guide.
• I have added test cases to cover the new code or have provided the test plan.

Test plan

[TheDarkUndoing]

@unknwon could you take a look at this, I want to make sure the changes effect the front end, is there a CICD workflow for me to test a build with my changes?

[unknwon]

@unknwon could you take a look at this, I want to make sure the changes effect the front end, is there a CICD workflow for me to test a build with my changes?

I just approved the CI jobs for this PR. Will take a closer at the code this week if all checks passed, thank you!

[codecov]

Codecov Report

Merging #7383 (a7f5c52) into main (689e71f) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7383   +/-   ##
=======================================
  Coverage   14.63%   14.63%           
=======================================
  Files         108      108           
  Lines       13880    13880           
=======================================
  Hits         2032     2032           
  Misses      11575    11575           
  Partials      273      273           

[TheDarkUndoing]

@unknwon could you take a look at this, I want to make sure the changes effect the front end, is there a CICD workflow for me to test a build with my changes?

I just approved the CI jobs for this PR. Will take a closer at the code this week if all checks passed, thank you!

Looks like the lint failed, I formatted and commited the change. Ready for CI job to be run again.

[unknwon]

Thank you!

[TheDarkUndoing]

This doesn't fix when a host has a port. The change I made would fix this. Your simpler solution does not fix this.

[TheDarkUndoing]

@unknwon Can we reopen this to properly address the bug?

[unknwon]

@TheDarkUndoing My bad, will take another look!

[TheDarkUndoing]

@unknwon my original change should address the issue

[unknwon]

This doesn't fix when a host has a port.

@TheDarkUndoing - what is an example of "a host has a port"?

[TheDarkUndoing]

For an example the gogs server is hosted on port 8080. The url for the host is {gogs host}:8080. I dont think there is a way to retain the port when parsing an ssh url with the way submodule.go is written. I think the baseUrl host does not retain the port number if it isn't hosted on port 80. It is written assuming that port 80 is the only port that would be hosting gogs

[unknwon]

For an example the gogs server is hosted on port 8080. The url for the host is {gogs host}:8080. I dont think there is a way to retain the port when parsing an ssh url with the way submodule.go is written. I think the baseUrl host does not retain the port number if it isn't hosted on port 80. It is written assuming that port 80 is the only port that would be hosting gogs

Yes, but how is your original version handles it? Here we are only dealing with ssh:// urls, there is no information to tell whether the http:// should have a port or not. 🤔

[TheDarkUndoing]

after reviewing my last change, I was wrong my changes dont effect that. I think we would need some kind of "global" variable or env that holds the proper host URL. That may already exist, I havent checked.

[unknwon]

after reviewing my last change, I was wrong my changes dont effect that. I think we would need some kind of "global" variable or env that holds the proper host URL. That may already exist, I havent checked.

Top off my head I think having a pre-defined "global" list is suboptimal, it is unpractical to know the host URL info ahead of time. I think extend the .gitmodules to include a non-SSH URL would be better (not final).

---

Since the new issue is unrelated to this thread, I'd recommend to file a new issue to address that (if we want to).