Fix F1 help link for error messages#46850
Conversation
| } | ||
|
|
||
| return string.Empty; | ||
| return $"https://msdn.microsoft.com/query/dev16.query?appId=Dev16IDEF1&l={CultureInfo.CurrentCulture}&k=k({GetId(code)});k(DevLang-csharp)&rd=true"; |
There was a problem hiding this comment.
I don't particularly like hardcoding dev16 into the compiler, I think any link would need to be VS-version agnostic.
What would happen when we add new error codes to the compiler that aren't yet doc'd?
There was a problem hiding this comment.
I don't particularly like hardcoding dev16 into the compiler, I think any link would need to be VS-version agnostic.
I don't think it's a big deal. I even don't know how it matters if it's dev16 or dev15 or whatever version. Both redirects to the correct page.
If the docs consolidated all error messages in the same directory, we can rely on it as a different link.
What would happen when we add new error codes to the compiler that aren't yet doc'd?
It will redirect to https://docs.microsoft.com/en-us/dotnet/csharp/misc/sorry-we-don-t-have-specifics-on-this-csharp-error
(Currently, it won't do that for all error messages. That will get fixed by dotnet/docs#20069
There was a problem hiding this comment.
i think this needs to go through doc team to know what links they want us to point at. @jinujoseph @kendrahavens @mikadumont do you know who would be a good contact there to ask about this?
There was a problem hiding this comment.
I don't think it's a big deal. I even don't know how it matters if it's dev16 or dev15 or whatever version. Both redirects to the correct page.
I'd prefer not to have any version at all. Errors from the C# compiler are not really version specific.
There was a problem hiding this comment.
@CyrusNajmabadi The docs team currently relies on f1_keywords metadata that exists in articles. Because the error messages documentation is scattered in two different folders currently, I don't think there is another way.
AFAIK, the docs team has an internal request to support a whole folder redirection, which will make it easy to have all error messages in one folder.
I'll let @BillWagner or @adegeo confirm that.
There was a problem hiding this comment.
OK, I will work with @kexugit about the new name and update you soon.
There was a problem hiding this comment.
We will deploy this change after the Ignite event ends.
There was a problem hiding this comment.
@Youssef1313 The change has been deployed. You can go ahead and update the PR. Thanks to @OsmondJiang 's team!
There was a problem hiding this comment.
Thanks a lot @kexugit and @OsmondJiang 🎉
https://msdn.microsoft.com/query/dev16.query?appId=Dev16IDEF1&l=EN-US&k=k(CS1001);k(DevLang-csharp)&rd=true
https://msdn.microsoft.com/query/roslyn.query?appId=roslyn&l=EN-US&k=k(CS1001);k(DevLang-csharp)&rd=true
At the time of writing this, the first link works, but the second doesn't. Any idea what I'm doing wrong?
There was a problem hiding this comment.
src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb
Show resolved
Hide resolved
|
@333fred Build is green. Can you review? |
333fred
left a comment
There was a problem hiding this comment.
LGTM (commit 17). @dotnet/roslyn-compiler for another review.
|
@dotnet/roslyn-compiler for a second review. |
|
Merged/squashed. Thanks @Youssef1313 ! |
Discussion started in dotnet/docs#20069 by @ChrisMaddock.
Fixes dotnet/docs#16448
Fixes #40863
Fixes #46829
Prefer squash/merge