crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.21k stars 1.61k forks source link

Add documentation tool #14707

Closed nobodywasishere closed 2 weeks ago

nobodywasishere commented 2 weeks ago

Implements #8276

Example connected to development version of vscode extension

image
nobodywasishere commented 2 weeks ago

I want to be able to provide documentation for macros but can't seem to figure out a way to do that currently, not as familiar with the compiler as I want to be. Something like this, which currently fails:

  it "find documentation of macro" do
    assert_documentation %(
      # ༓foo docs
      macro foo
        def bar
        end
      end

      f‸oo
      bar
    ), check_lines: false
  end
straight-shoota commented 2 weeks ago

Thank you for this contribution. But I think we'll need to talk a bit more about the details of this feature though, before we can commit to a specific implementation. We ask all major feature contributions to go through a discussion of the general design first.

8276 is merely a thin idea with very little details. That's hardly a design spec.

And actually I don't think this PR has much in common with that idea. I understand #8276 is mainly a command line interface to query contents from the documentation dump (index.json). While this is a tool for IDE integration.

IMO the main functionality of this PR is identifying the target def (or path) for a given source code location. Fetching the documentation for that is just a small extra on top, but wouldn't be necessary. Given the name of the target def/path, it's easy to fetch the documentation from index.json. Considering that, I believe it would be quite reasonable to achieve the same result with a combination of crystal tool implementations to query the target def and crystal doc to fetch the focumentation. This of course requires two separate compiler instances, doing double work (and taking double time). So it's not ideal. https://github.com/crystal-lang/crystal/issues/14705 would be an improvement for that.

Thinking about crystal tool implementations, maybe we could add an alternative output format for documentation to this command? It seems like this would be pretty similar to the proposal in this PR except that it's not a completely new tool but enhancing an existing one.

nobodywasishere commented 2 weeks ago

I think that makes sense to me. Apologies for going through with an implementation before having more discussion.

I'll try seeing how well crystal tool implementations + crystal doc works and report back, don't want redundant tooling if we can't help it. Otherwise a --with-doc flag for the implementations command should be fairly easy to add.

straight-shoota commented 2 weeks ago

No need to apologize. The purpose of having a discussion first is to protect you from wasting effort on working out an implementation if it gets rejected.