Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String Links 404 #794

Closed
felix-alonso opened this issue Feb 14, 2023 · 15 comments
Closed

String Links 404 #794

felix-alonso opened this issue Feb 14, 2023 · 15 comments
Labels
good first issue Tasks that are good for newcomers to the codebase help wanted Tasks that are up for grabs!

Comments

@felix-alonso
Copy link
Contributor

The links to System.String and System.Text.RegularExpressions are 404'ing on this page:

https://fsharp.github.io/fsharp-core-docs/reference/fsharp-core-stringmodule.html

Seems like the format of the link changed:

https://learn.microsoft.com/en-us/dotnet/api/system.string?view=netcore-3.1

@baronfel
Copy link
Collaborator

it looks like this has (partially) already been fixed in FSharp.Formatting - the URL generation uses docs.microsoft.com: https://github.dev/fsprojects/FSharp.Formatting/blob/a9ecebc3db319f2a3d07f841755ffc61bbf936df/src/FSharp.Formatting.ApiDocs/GenerateModel.fs#L950.

However, this should be updated to use learn.microsoft.com instead as a root.

Separately, the https://github.com/fsharp/fsharp-core-docs repo should update to a more recent FSharp.Formatting and republish the docs to get the fix.

@nojaf
Copy link
Collaborator

nojaf commented Feb 15, 2023

Hi Felix, are you interested in contributing here?

@nojaf nojaf added help wanted Tasks that are up for grabs! good first issue Tasks that are good for newcomers to the codebase labels Feb 15, 2023
@felix-alonso
Copy link
Contributor Author

felix-alonso commented Feb 18, 2023

@nojaf,

Possibly! I'm just getting started with F#. How could I help?


When I clone and build the repo, I'm getting an MS Build exception.

image

I'm running:

OS:

Linux 5.10.0-20-amd64 #1 SMP Debian 5.10.158-2 (2022-12-13) x86_64 GNU/Linux

.NET: v 7.0.102

IDE:

Version: 1.75.1
Commit: 441438abd1ac652551dbe4d408dfcec8a499b8bf
Date: 2023-02-08T21:35:30.018Z
Electron: 19.1.9
Chromium: 102.0.5005.194
Node.js: 16.14.2
V8: 10.2.154.23-electron.0
OS: Linux x64 5.10.0-20-amd64
Sandboxed: No

Ionide.Ionide-fsharp: v7.4.2


Am I doing anything obviously wrong?

@felix-alonso
Copy link
Contributor Author

I think I've gotten past the build issue. I can run the documentation locally. I'm just not sure how I can verify that I've fixed the external documentation link.

I see that this Classic XML Doc Comments contains an external link, but it uses the other URL format. When I click on the link it redirects to the new URL format

Current: https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/xmldoc/
Redirect: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/xmldoc/

image

When I look through the project it seems like there are half a dozen other references to that URL.

image

Should change all of the instances of docs.microsoft.com/ to learn.microsoft.com/?

@nhirschey
Copy link
Collaborator

nhirschey commented Feb 18, 2023

Thanks for the help!

I'm just not sure how I can verify that I've fixed the external documentation link.

The fsharp core docs are here https://github.com/fsharp/fsharp-core-docs

You’d want to build that repo’s docs using the fsdocs tool built with your PR. You can build those docs following the Build Steps on the readme.

The key thing would be to clone the repos as described in the fsharp-core-docs build steps. Then make your proposed changes in the FSharp.Formatting repo and build fsdocs with your proposed changes. Then do

(from 'fsharp-core-docs')
FSharp.Formatting\src\FSharp.Formatting.CommandTool\bin\Release\netcoreapp3.1\fsdocs.exe watch --sourcefolder fsharp

It’ll pop up a browser with the fsharp docs and you can test the URLs.

@felix-alonso
Copy link
Contributor Author

@nhirschey,

I've been able to mostly follow the build steps. I had to run the following to spin up the website:

(start in 'fsharp-core-docs')
dotnet restore FSharp.Core

(make 'fsharp-core-docs/fsharp' and 'fsharp-core-docs/FSharp.Formatting' )
git clone https://github.com/dotnet/fsharp --depth 1 -b main
git clone https://github.com/fsprojects/FSharp.Formatting --depth 1

(build 'fsharp-core-docs/fsharp'; didn't take noVisualStudio flag)
pushd fsharp
./build.sh
popd

(build 'fsharp-core-docs/FSharp.Formatting'; didn't take build flag)
pushd FSharp.Formatting
./build.sh
popd

(from 'fsharp-core-docs')
FSharp.Formatting/src/fsdocs-tool/bin/Release/net7.0/fsdocs watch --sourcefolder fsharp

However, it doesn't seem to pick up any modules in fsharp project. See the docs below:

image

Similarly, clicking on any of the documentation links it just complains that the connection was reset:

image

image

Any clue what might be causing this?

@nojaf
Copy link
Collaborator

nojaf commented Feb 20, 2023

Hello @felix-alonso, I just tried these steps on Windows and could view the API docs there.
So, maybe this wasn't designed to work on anything else than Windows. The workflows seem to support this theory.
I can help you with the fsharp-core-docs repository once the fix is available in fsdocs.

Should change all of the instances of docs.microsoft.com/ to learn.microsoft.com/?

Yes, I believe so, if possible maybe refactor things a bit so that we use "learn.microsoft.com/" as a single constant.

Thanks again for looking into this!

@nojaf
Copy link
Collaborator

nojaf commented Feb 20, 2023

I just found out that the links to System.String and System.Text.RegularExpressions are actually coming from:

https://github.com/dotnet/fsharp/blob/979fa51c3440c3e32b56c579b030b9df5636f9f1/src/FSharp.Core/string.fsi#L10-L11

So, this would need a fix in dotnet/fsharp as well.

@felix-alonso
Copy link
Contributor Author

@nojaf,

I've marked this PR as ready for review. Thank you for your guidance in this process!

#797

@felix-alonso
Copy link
Contributor Author

felix-alonso commented Feb 20, 2023

I just found out that the links to System.String and System.Text.RegularExpressions are actually coming from:

https://github.com/dotnet/fsharp/blob/979fa51c3440c3e32b56c579b030b9df5636f9f1/src/FSharp.Core/string.fsi#L10-L11

So, this would need a fix in dotnet/fsharp as well.

Looks like these are the correct links:

  • System.String
    • Before: http://msdn2.microsoft.com/en-us/library/system.string.aspx
    • After: https://learn.microsoft.com/en-us/dotnet/api/system.string
  • System.Text.RegularExpressions
    • Before: http://msdn2.microsoft.com/library/system.text.regularexpressions.aspx
    • After: https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions

So the change is something like s|msdn2.microsoft.com/en-us/library|learn.microsoft.com/en-us/dotnet/api|g and removing the .aspx.

@nojaf
Copy link
Collaborator

nojaf commented Feb 21, 2023

@felix-alonso thanks! I've updated all the links at the compiler side: dotnet/fsharp#14774
Once, that is merged we can try and update https://github.com/fsharp/fsharp-core-docs

@felix-alonso
Copy link
Contributor Author

@nojaf,

Does this mean I get to put FSharp core contributor on my resume? 🤣

Thank you!

@nojaf
Copy link
Collaborator

nojaf commented Mar 13, 2023

@felix-alonso I believe this problem is resolved now. Please confirm.

@felix-alonso
Copy link
Contributor Author

@nojaf,

It's looking good! Thank you so much!

@nojaf
Copy link
Collaborator

nojaf commented Mar 15, 2023

Thanks for confirming.

@nojaf nojaf closed this as completed Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Tasks that are good for newcomers to the codebase help wanted Tasks that are up for grabs!
Projects
None yet
Development

No branches or pull requests

4 participants