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

Adds methods for cross-type links #330

Merged
merged 15 commits into from
Oct 14, 2015
Merged

Adds methods for cross-type links #330

merged 15 commits into from
Oct 14, 2015

Conversation

edhzsz
Copy link
Contributor

@edhzsz edhzsz commented Jul 8, 2015

As discussed in Issue #52
This pull request allows to create links to other types in the assembly using markdown style links with the name of a type. It can be full name or simple name:

module MyModule =
  type MyType = int

  /// This returns a [MyModule.MyType].
  let f x :MyType = x * 2

  /// This returns a [MyType].
  let f x :MyType = x * 2

Work in progress. Please review.

TODO:

  • Support cross-type links using Markdown links and full name of the type
  • Support cross-type links using Markdown links and simple name of the type
  • Support cross-type links when inline code (using backticks) contains only the (simple or full) name of a type
  • Add examples of usage of cross-type links to the metadata.fsx docs
  • Add warning when a link cannot be resolved due to duplicate possibilities.

Adds basic test for linking types.
Adds methods to collect all indirect links, find the type references and
add the links to the found types.
@tpetricek
Copy link
Member

This is looking great! I like the trick where you're finding all links and add the definitions for them, that's clever :-).

I wonder what would be the most common uses for this. I had a quick look at some of the Deedle docs and that is mostly using the backtick (inline code) when referring to other functions/types/modules. So obviously, getting it to work for Deedle would be one nice thing.

I'm not sure what other features would make the most sense. I'll ask for feedback on Twitter :)

edhzsz added 4 commits July 27, 2015 23:55
* master:
  Release 2.10.0
  Fix bug in CSharpFormat
  paket update && change logging setup logic to include Yaaf.FSharp.Scripting.
  fix path of documentation file.
  Switch to a logging Framework (System.Diagnostics.TraceSource) for better configuration.
Added more test cases for
- Types with duplicated names
- Inexistent types

Restricted links to be only internal because if the type is not found in
`UrlMap.ResolveCref`, the url returned is a URL to msdn documentation.
@edhzsz
Copy link
Contributor Author

edhzsz commented Jul 29, 2015

I haven't saw any feedback, so I continued with my implementation. 😄

I have added the possibility to link using only the type simple name instead of the full type name.
This causes a problem when there are types with duplicated simple names. I decided to leave the link as undefined in this case.

I am not sure if links to external types should be supported. I believe they should not.

With this, I am near to be able to create links when backtick is used. I will restrict myself to types and modules in this pull request and I will try to add links to functions later.

@edhzsz edhzsz changed the title [WIP] Adds methods to link other types using type full name [WIP] Adds methods for cross-type links Jul 29, 2015
* master:
  Specify range for FCS in NuGet (sort of fix #337)
  Fix broken script
  Update release notes
  Wrap logging setup in try catch
  paket update && fix compilation.
@matthid
Copy link
Member

matthid commented Sep 20, 2015

Looks good to me. Just one note: If you cannot resolve the name due to duplicate possibilities it might make sense to notify the user with a logging entry (warning?).
Ideally we use the same symbol the compiler would use in the same context, which is not possible :(.

I am not sure if links to external types should be supported. I believe they should not.

In <see cref='s' /> links I decided to link against MSDN if the link could not be resolved internally, which is what Visual Studio is doing when pressing F1? For simple names I tend to agree with you though, as it is quite difficult to find the correct type (without the information a compiler has at this position).

Again very nice! I think I will update the cref resolution to use this as well once you are finished. Thanks.

Thinking about this further: Maybe it would make sense in the future to generate the reference documentation before the simple markdown files. This should enable us to replace links in the regular documentation as well (Of course not part of this PR, but a lot of this code could be reused to make this happen)

edhzsz added 6 commits October 4, 2015 18:10
* master:
  release notes.
  paket update
  bump to 2.11.0-alpha2. FullName can fail.
  Add Gitter badge
  General stabilization. Ignore when we cannot handle a module, type or a single member and write an error, workaround for #271 (and future FCS bugs).
  add overload that takes a sequence, fixes #49
  create and push tag
  disable ReleaseBinaries (to not push an alpha there). Do we still need this? Change Url to be able to push without password.
  Update to 2.11.0-alpha1
  Update Yaaf.FSharp.Scripting (now uses the 4.5 reference assemblies). Fixes crash with C# extension methods when the assembly containing them was build against net45. Fixes #201. (Might fix) References #328, #325, #139
  Make [omit] work when markDownComments is false. Fail early when we cannot get the attributes of the current member.
@edhzsz
Copy link
Contributor Author

edhzsz commented Oct 14, 2015

Finally had time to continue working on this. Basically I just finished the implementation.
I am adding the docs and I will add the warning when the link could not be created due to duplicated possibilities.
I also believe that it would be better to generate the links earlier in the process, I will finish this and I will give it a try.

@edhzsz
Copy link
Contributor Author

edhzsz commented Oct 14, 2015

Good to go!... I hope. 😄

matthid added a commit that referenced this pull request Oct 14, 2015
[WIP] Adds methods for cross-type links
@matthid matthid merged commit 044c3d5 into fsprojects:master Oct 14, 2015
@matthid
Copy link
Member

matthid commented Oct 14, 2015

@edhzsz edhzsz changed the title [WIP] Adds methods for cross-type links Adds methods for cross-type links Oct 15, 2015
@edhzsz edhzsz deleted the links_to_other_types branch October 15, 2015 06:41
@matthid matthid mentioned this pull request Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants