port fsih to fsi as a hash directive

[dawedawe]

Description

This is a port of fsih to fsi itself as a hash directive.

I think using a hash directive for this makes the most sense but as we use the normal parser for them it also means we have to wrap the expression in parentheses. I hope that's an acceptable tradeoff.
If you want to give this a try, you have to copy the FSharp.Core.xml manually to the fsi artifacts folder. The build process doesn't do it. Later, in the SDK distribution, the xml file will be there as far as I can see.

Checklist

• Test cases added

• Performance benchmarks added in case of performance changes

• Release notes entry updated:

  Please make sure to add an entry with short succinct description of the change as well as link to this pull request to the respective release notes file, if applicable.

  Release notes files:

  • If anything under src/Compiler has been changed, please make sure to make an entry in docs/release-notes/.FSharp.Compiler.Service/<version>.md, where <version> is usually "highest" one, e.g. 42.8.200
  • If language feature was added (i.e. LanguageFeatures.fsi was changed), please add it to docs/releae-notes/.Language/preview.md
  • If a change to FSharp.Core was made, please make sure to edit docs/release-notes/.FSharp.Core/<version>.md where version is "highest" one, e.g. 8.0.200.

  Information about the release notes entries format can be found in the documentation.
  Example:

  • More inlines for Result module. (PR #16106)
  • Correctly handle assembly imports with public key token of 0 length. (Issue #16359, PR #16363)
    *while! (Language suggestion #1038, PR #14238)

  If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/8.0.400.md

[vzarytovskii]

Nice, I'm fine with having it shipped by default, it will need tests though (simple baseline which verifies couple of outputs).

And an approval from @KevinRansom

[baronfel]

From a usability perspective, is there any auto complete support for hash directive contents? If so, users could dot-into the expression they want docs for.

[psfinaki]

Hey Dawe, I think this would be a great addition to FSI, worth spreading a word about it once it's in.

A note on errh clean code, would you want to move the new module to a separate file or something like this? fsi.fs is already very fat and seems like a rather easy target for extraction.

[vzarytovskii]

Should probably use shims here for the filesystem.

[smoothdeveloper]

Considerations:

• would it be better done as a FSI extension through #r "fsih: List.map"?
• for terminal output, it would be possible to pretty print a bit the output, using brighter coloring for headings such as "Description:", "Parameters:", etc. as well as bulleted items, it will improve the legibility of the output; or maybe in markdown format (the IDE can then figure out how to pretty print when handling the evaluation)
• the code samples would also have syntax colouring (probably not semantic coloring that requires type checking, but whatever we could have), but this is probably more challenging item

Disregarding this, it already sound like nice feature that will speak to users of python REPL and other similar environments 🙂.

[vzarytovskii]

Considerations:

• would it be better done as a FSI extension through #r "fsih: List.map"?

It's not a dependency manager related functionality, so probably not(?). There's no such thing as "FSI extension", #r is a dependency manager functionality/extension. And if done via DM, it will be quite verbose and confusing.

• for terminal output, it would be possible to pretty print a bit the output, using brighter coloring for headings such as "Description:", "Parameters:", etc. as well as bulleted items, it will improve the legibility of the output; or maybe in markdown format (the IDE can then figure out how to pretty print when handling the evaluation)

In this case, we will need additional testing for it for different $TERMs, and that it's not breaking the output of interactive and when used as API.

• the code samples would also have syntax colouring (probably not semantic coloring that requires type checking, but whatever we could have), but this is probably more challenging item

Also would require much more testing with both non-terminal environments as well as all popular emulators and shells.

[brianrourkeboll]

From a usability perspective, is there any auto complete support for hash directive contents?

There is autocomplete in VS for filepaths inside the "…" after #I "…", #r "…", etc., in an .fsx file.

But I think I'm a bit thrown off by the #h "expr";; syntax myself.

What is the primary motivation for using a hash directive?

1. For symmetry with #help?
2. So that this feature itself shows up in #help (which currently only shows help for itself and other FSI directives)?
3. To ensure that the user can't accidentally shadow the ability to access this functionality by defining their own value h?

(1) doesn't feel super-compelling to me, since all the other FSI directives, including #help, operate on the level of (i.e., refer to or affect) the FSI session, not on expressions within the session.

If (2), then I think we could just add the help for h to the information that #help shows. We could also add help for the existing fsi object while we were at it.

If (3), then what about just adding an h method to the existing fsi object, which is less likely to be accidentally shadowed than h on its own?

fsi.h List.map;;

[dawedawe]

Hey Dawe, I think this would be a great addition to FSI, worth spreading a word about it once it's in.

A note on errh clean code, would you want to move the new module to a separate file or something like this? fsi.fs is already very fat and seems like a rather easy target for extraction.

Thanks :)
Yeah, I thought about that but then decided to follow the existing all-in-one-file style of fsi for consistency.
But sure, if y'all are okay with a dedicated file, I'll definitely split it off.

[dawedawe]

What is the primary motivation for using a hash directive?

1. For symmetry with `#help`?

2. So that this feature itself shows up in `#help` (which currently only shows help for itself and other FSI directives)?

3. To ensure that the user can't accidentally shadow the ability to access this functionality by defining their own value `h`?

My motivation was an uniform distribution among all three points.
Avoiding shadowing is a must-have for me.

You're making a strong argument for a new fsi.h function to provide the feature. But users should be able to discover it in the output of #help. So yeah, as long as the team is cool with it, that would also be my preferred way forward.

[smoothdeveloper]

#r is a dependency manager functionality/extension. And if done via DM, it will be quite verbose and confusing.

I concede it might sound unatural, but #r stands for "reference" (looking for "reference documentation"), and #r extensions can/do print to the output. It would need 0 parser changes.

@brianrourkeboll suggestion of fsi.h sounds much better in any case and very close to current fsih package.

I also prefer it to hash directive and quoting the expression.

Are there special events that FSI session will trigger?

Can such events be done from a type extension over FSharp.Compiler.Interactive.InteractiveSession?

This could help extending IDEs in terms of how the output is parsed / displayed (with the idea of markdown or pretty printing strategies that could be implemented on top, rather than raw parsing of stdout).

[dawedawe]

Okay, I moved to fsi.h as an user interface and added a printer for the output processing. Meaning, the record option containing the various documentation pieces is available as it afterwards.

The downside is, that the FSharp.Compiler.Interactive.Settings project (which hosts the fsi aka InteractiveSession) can't access the shim filesystem.

[dawedawe]

I think this is in an okay state now but I don't know how to add tests for it.
Is there some testing facility that can actually access the fsi object?
I tried FSharpScript from ScriptHelpers.fs but it seems like it can only do hash directives and normal code, no member calls of fsi.

[baronfel]

Maybe just test the implementation directly with various Exprs? I'm not sure there's value in testing that you can call fsi.h directly, it's more important that the behavior of the expression-traversal be covered by tests.

[edgarfgp]

@vzarytovskii @KevinRansom Can we please have a review here ?