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

Include material from clojuredocs.org in function documentation #689

Closed
crispinb opened this issue Jul 27, 2020 · 42 comments · Fixed by #1363
Closed

Include material from clojuredocs.org in function documentation #689

crispinb opened this issue Jul 27, 2020 · 42 comments · Fixed by #1363

Comments

@crispinb
Copy link
Contributor

The clojuredocs.org documentation for clojure core is really handy. The examples are invaluable given how terse clojure docs can be.

It would be great to have access to the clojuredocs for a given function - whether added to the 'show hover' doc output used in the current implementation, or via a new command.

@bpringe
Copy link
Member

bpringe commented Jul 27, 2020

I think this is a good idea.

@PEZ
Copy link
Collaborator

PEZ commented Jul 27, 2020

Love it! I think the Orchard has support for it.

@zackteo
Copy link

zackteo commented Aug 4, 2020

I wonder if the current pop up documentation will work well with this tho. In CIDER another window will open up, this works well because some of the functions in clojuredocs.org have a lot of examples

(I use CIDER but a friend of mine is getting into Clojure through Calva)

@crispinb
Copy link
Contributor Author

crispinb commented Aug 4, 2020

It might be a bit unwieldy with the current popup, true enough. Hard to tell for sure without seeing it, but the tradeoff might be worth it for those of us who make use of clojuredocs.

@bpringe
Copy link
Member

bpringe commented Aug 4, 2020

Couple of ideas here:

  1. We use a webview to show the clojuredocs page for the particular function. Here the webview would just be used to show html, and nothing more, so it would not give us trouble like the old repl window.

  2. However, we could also use an editor like the new repl window, so all the examples from clojuredocs would be immediately executable from the editor. (Have not thought this through further yet - might be more trouble than it's worth - are executable examples really necessary/helpful? The output is usually shown with the examples anyway.)

@PEZ
Copy link
Collaborator

PEZ commented Aug 4, 2020

I like number 2.

We don't need to populate the pop-up with this. We could have commands that paste it to the output/repl.

@zackteo
Copy link

zackteo commented Aug 4, 2020

Sorry maybe I'm mistaken but don't both 1 and 2 work out? That is I'm assuming both options it will open in another window :o ? I agree with @PEZ about not populating the pop-up.

I don't think the examples need to be immediately executable. Just need to be able to seamlessly check the docs, close it and continue with what you are working on. That's my thoughts

@bpringe
Copy link
Member

bpringe commented Aug 5, 2020

1 and 2 are just different approaches to the same thing, but both would open another window, yes. I find this arguably the same as just using a browser window though, if it has to be another window in the editor.

I do worry though that if we were to use the output window that could interrupt a person's workflow. Someone might be working on some code in the repl window, and if they run the command to look something up in that window their code will be interrupted by the clojuredocs output.

@PEZ
Copy link
Collaborator

PEZ commented Aug 5, 2020

We can paste it into an untitled editor window with the extension repl-file. It would make it easy to do whatever you like with the information and examples, including evaluate them.

@glebovmaksim
Copy link

It's good enough for me to just show the output in a webview. If I want to evaluate example I can just copy it to a regular repl window (and as @bpringe said, the examples usually have the output in the comments already). I'm afraid that the repl-file solution will bring a lot of questions (if it is a regular repl-file, in which namespace will the evaluation work? can we close the non-empty untitled file without confirmation dialog? etc). But maybe I just misunderstood the idea.

@PEZ
Copy link
Collaborator

PEZ commented Aug 6, 2020

I think you are understanding it. But you might not be as reluctant as I am to bring in webviews. 😄 And you might not be as keen as I am on experimenting with example code. That is the first thing I do when I try to understand things.

@bpringe
Copy link
Member

bpringe commented Aug 6, 2020

I mostly agree with @glebovmaksim here in terms of the added complexity / questions a repl-file would bring, but if we start using "fiddle files" in the future as you've mentioned @PEZ, we might have a better solution/workflow around those.

I do think though that using a webview purely as read-only is significantly less troublesome than what was done before with the old repl window, and makes sense here (ignoring being able to edit/run the examples), similar to a markdown preview webview.

Editing/evaluating the examples in-place sounds fun/cool, but again, questions. The results would be printed at the bottom of all the examples, not the particular example you evaluated, making it hard to work with the examples, which then leads to copying/pasting anyway if wanting to play with the examples.

I think in most cases of someone looking at clojuredocs, they do not try to run the examples, but use them merely as a quick "aha" and move on with their code.

Lastly, a quick incremental solution would be to add a command or link in the hover/description that opens a browser to the clojuredocs page. I honestly find flipping between a browser window and vscode about the same amount of effort/trouble as if the clojuredocs page were opened in another window in vscode.

Basically, if we aren't putting the clojuredocs material in a hover, I find it almost pointless not to just use a browser outside of vs code.

@crispinb
Copy link
Contributor Author

crispinb commented Aug 6, 2020

I think in most cases of someone looking at clojuredocs, they do not try to run the examples, but use them merely as a quick "aha" and move on with their code.

I think this is half right, but it's the right half! As a current clojure learner, I might well spend some time perusing clojuredocs for a group of functions, perhaps copying bits out to a scratch project to play with etc. But the web site is fine for that. It's a quite different activity from a quick lookup in an editing session - there what the user wants is to briefly (a) glance at and then (b) dismiss the info. Although VS Code might not provide the perfect infrastructure for that (yet?), the hover is the closest approximation IMO.

Personally (and others' workflows differ of course) opening up a separate OS window (browser) or editor frame (whether web view or standard editor) offers just a bit more friction than this kind of quick lookup demands.

a quick incremental solution would be to add a command or link in the hover/description that opens a browser to the clojuredocs page.

Yep, this is how IntelliJ does it (a modal popup, with a further command to launch an external browser window or other document viewer).

In an ideal world that command/link would be configurable - eg. to launch a browser or Zeal/Dash (or similar) if installed, but that may be a bridge too far at this stage.

In summary, I think the logical distinction between "look up while editing" and "single-minded reading of documentation" is worth maintaining.

@glebovmaksim
Copy link

Good point, @crispinb!

Yep, this is how IntelliJ does it (a modal popup, with a further command to launch an external browser window or other document viewer).

It seems like it has more than that. I wonder if this can be implemented in a similar way in VSCode...
Screenshot from 2020-08-07 01-17-54

@PEZ
Copy link
Collaborator

PEZ commented Aug 7, 2020

I think we can do that. How is this dialog brought up in Cursive?

@glebovmaksim
Copy link

It appears on hover. The same way as function signature / docstring in Calva.

@crispinb
Copy link
Contributor Author

crispinb commented Aug 7, 2020

.. or via keyboard shortcut (ctrl-Q on linux). And once it's up there's another keyboard shortcut (F4) to jump to the documented source code. Some IntelliJ language plugins (but not Cursive IIRC) offer a further shortcut to launch external documentation (eg. a web page, or open a documentation browsing app like Dash or Zeal).

@bpringe
Copy link
Member

bpringe commented Dec 24, 2020

So it looks like we came back to adding these to the hover? 😄 I really don't see an issue with that.

@PEZ PEZ removed the enhancement label Feb 10, 2021
@bpringe bpringe mentioned this issue Apr 12, 2021
3 tasks
@shinichy
Copy link

I really miss this feature when I switch from Cursive to Calva. Looks like the hover is implemented by clojure-lsp. Then I guess clojure-lsp needs to implement it?
https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/feature/hover.clj#L32

@bpringe
Copy link
Member

bpringe commented Sep 19, 2021

@shinichy, when a REPL connection exists, the hover comes from Calva / the REPL. Otherwise, it comes from clojure-lsp. Even with the hover that comes from clojure-lsp, I think Calva can enrich it to provide the clojuredocs info.

@shinichy
Copy link

@bpringe Thanks! Now I found the hover implementation in Calva. I'll try to implement this feature when I have time.
https://github.com/BetterThanTomorrow/calva/blob/dev/src/providers/hover.ts#L8

@bpringe
Copy link
Member

bpringe commented Sep 21, 2021

@shinichy Awesome! If it's easier to just implement the feature for the REPL (the hover provider) then that could be a good first step. However, you could probably isolate the implementation and use it to also enrich the clojure-lsp hover.

return next(document, position, token);

It seems there's an API (https://www.programmableweb.com/api/clojuredocs-rest-api), but I don't see any documentation for it... has anyone else used this API?

I've found this: https://github.com/dakrone/cd-wsapi. Maybe someone knows how Cursive gets the data?

@shinichy
Copy link

@bpringe The API doesn't seem to work. What I found is https://clojuredocs.org/clojuredocs-export.json. Looks like the EDN version is also available https://github.com/clojure-emacs/clojuredocs-export-edn and Orchard uses it https://github.com/clojure-emacs/orchard/blob/master/src/orchard/clojuredocs.clj. Since Calva uses Typescript, I think we can just parse the JSON file.

I think there are some options to implement this. I'm wondering which one is good to go.

  1. Bundle the file with Calva
    Pros: Easy to implement
    Cons: Increase the extension size (The JSON file is 3.2MB)
  2. Download and cache it in memory when invoking the hover (Orchard)
    Pros: Easy to implement
    Cons: It may block the hover when the internet is slow
  3. Download and cache it in background when opening a Clojure file
    Pros: Better UX?
    Cons: Not so easy to implement?
  4. Create a UI to download in settings and notification (Cursive)
    Pros: Better UX?
    Cons: Not so easy to implement?

@PEZ
Copy link
Collaborator

PEZ commented Sep 21, 2021

I still think this should go into the output window. The hover is tricky to navigate. If there is any code at the prompt we can take care of inserting that code. The output window is ready to let you evaluate or copy any of the examples.

@PEZ
Copy link
Collaborator

PEZ commented Sep 21, 2021

As for implementation. It is probably easiest to implement the cider-nrepl ops clojuredocs-refresh-cache (we can call that one at REPL startup) and clojuredocs-lookup. If we want it to be a static service, then adding similar ops to clojure-lsp makes sense. (This might make sense regardless, since then other clojure-lsp enabled apps will benefit.)

@bpringe
Copy link
Member

bpringe commented Sep 22, 2021

CC @ericdallo ^

@PEZ,

I still think this should go into the output window.

So if putting the docs in the output window, would it happen via a command, rather than hovering?

The hover is tricky to navigate. If there is any code at the prompt we can take care of inserting that code.

Can you expand more on these two points?

The output window is ready to let you evaluate or copy any of the examples.

I think usually people will not want to do this. From the hover they can also copy the example.

There's also the risk of it breaking someone's output flow. Maybe they have a series of results in the output window as they are trying things out from the editor, and they want to keep those results near each other for context. If they then want to see the clojuredocs to reference something, then try something else, then the results will now be intermingled with/broken up by the clojuredocs in the output window.

Some people may also like to work with the output window closed at all times or most of the time, and might not like that they have to use it to see clojuredocs.

I, personally, would prefer the clojuredocs to be in the hover.

@ericdallo
Copy link
Contributor

ericdallo commented Sep 22, 2021

Does someone has an gif or screenshot on how that works on Cursive?
It does looks something only easily available via cider-nrepl, It'd be good to understand how cursive does that as well

@bpringe
Copy link
Member

bpringe commented Sep 22, 2021

@ericdallo See this screenshot. I'm not sure how Cursive implements it though.

Regarding the hover/output-window discussion, we can also have a button/link in the hover for sending an example, or the whole doc content to the output window from the hover, for those that might want that, though I'd consider this not important for the initial implementation. I just wanted to give an example of still utilizing the output window if we went with a hover.

@ericdallo
Copy link
Contributor

ericdallo commented Sep 22, 2021

@bpringe debugging how cider-nrepl finds the docs, it just uses orchard docs feature, I think we could add that feature to clojure-lsp, a POC would be great, feel free to open an issue on clojure-lsp about that

@shinichy
Copy link

@PEZ I also would prefer to show the clojuredocs in the hover. But thanks for suggesting the solution! It's good to know that cider-nrepl already has it.

@ericdallo I created a feature request on clojure-lsp clojure-lsp/clojure-lsp#571.

@PEZ
Copy link
Collaborator

PEZ commented Sep 23, 2021

@bringe:

So if putting the docs in the output window, would it happen via a command, rather than hovering?

It would be a command and the hover can have a button for executing it.

The hover is tricky to navigate. If there is any code at the prompt we can take care of inserting that code.

Can you expand more on these two points?

Tricky to navigate: The hover is a tiny little window that goes away if you move the mouse outside it (or just touch the mouse pointer. when brought up by keyboard). Scrolling it is not convenient at all. The ClojureDocs examples are sometimes plenty and sometimes pretty big and wide. The hover is not a good place for such content. And if you just want to have some examples in display while coding something, then the hover won't work for this.

Code at the prompt: You might very well be working with something in the output window, since it is also a REPL prompt (something we can challenge in a separate issue). If we print stuff to the window, it would be polite to take care of any input the user has entered at the prompt and move it last after the printing of examples is done.

The output window is ready to let you evaluate or copy any of the examples.
I think usually people will not want to do this. From the hover they can also copy the example.

I think this is a great benefit when working in a REPL powered language. Personally I often copy examples from the ClojureDocs site and experiment with them in Calva. If we print examples to the REPL window (or in a separate file, or as (comment ...)) the examples are placed directly in a REPL context, as it should be. Copying something more than a few words from the hover is not easy at all. And we don't have Paredit in there to assist.

As for keeping recent output near. That's the case with all printing to the output window. I don't think it is a very important consideration. And also, this is only if we print it to the output window. We can print it in several different places that are better than in the hover.

Some people may also like to work with the output window closed at all times or most of the time, and might not like that they have to use it to see clojuredocs.

Well, it would open, they would see the docs. They can experiment with it or whatever. They can then close it. What's not to like? 😄This would probably be the workflow I would choose. I seldom run with the output window open. And sometimes I would leave the output open to the side to have it in view while coding up something.

As long as we implement the infra for this right we can always give the user options. A setting if it should go in the hover automatically. (We have at least two in this thread that would want to enable that.) Some different commands for printing docs as comments or separate window or the output window.

@bpringe
Copy link
Member

bpringe commented Sep 24, 2021

Thanks for the explanation, @PEZ. It's possible the hover in VS Code is harder to use for scrolling and copying than the one in Cursive, for example. I can't say I've tried to do much of that so I wouldn't know the issues.

If we implement this to where it's easy to send the output to different places, which I think is appropriate whether we used different places or not to begin with, then maybe we just stick with one place to begin with, and maybe that place can be the preference of whoever creates the PR? Just and idea.

But then again, if it's easy enough to send to another place, we might as well do the hover and output window.

@PEZ
Copy link
Collaborator

PEZ commented Sep 25, 2021

maybe that place can be the preference of whoever creates the PR?

Not fond of that way of doing things. I'm all for ”scratching your own itch”, but when it comes to Ux choices I think we need to make the call being the stewards.

But then again, if it's easy enough to send to another place, we might as well do the hover and output window.

Or just the output window, with a button in the hover. That said, and by all means, I would be happy to try a build where the examples are put in the hover. I could be wrong about how horrible that would be. 😄

@bpringe
Copy link
Member

bpringe commented Sep 26, 2021

but when it comes to Ux choices I think we need to make the call being the stewards.

Fair enough. I believe the stewards may disagree here, though 😄 . Since changing the location in the future or adding options seems to be fairly easy, we could go with your preference, @PEZ. I think we can both agree that our assumptions about putting the docs in the hover may be wrong though (it could be better than what you think, or worse than I think), and it's worth trying out at least.

@ericdallo
Copy link
Contributor

ericdallo commented Oct 17, 2021

Done on clojure-lsp, available on next clojure-lsp release :)

It's under a flag :hover :clojuredocs that is true by default as this is the kind of feature that makes sense be available in a IDE/LSP just like Cursive does on hover. Calva can still disable it and implement it itself, but IMO this is something that makes sense be available on hover feature by default.

Example on Calva:
image

@ericdallo
Copy link
Contributor

ericdallo commented Oct 17, 2021

@bpringe @PEZ Maybe with this feature on clojure-lsp, Calva should allow users to somehow choose whether to always use clojure-lsp hover documentation instead of the repl one that IMO is not as much detailed as clojure-lsp one 😛 ?
It is just that I can see users want to have always clojuredocs available even with the REPL running.

@PEZ
Copy link
Collaborator

PEZ commented Oct 17, 2021

Well, I still don't think the hover is the best place for clojure-docs examples. But that's me. We could maybe make the clojure-lsp hover something that users can opt to always have enabled, I guess.

@ericdallo
Copy link
Contributor

Well, I still don't think the hover is the best place for clojure-docs examples. But that's me. We could maybe make the clojure-lsp hover something that users can opt to always have enabled, I guess.

I tested on both Emacs/lsp-mode and Calva and it looked to me really good, but that's my opinion too 😅
I'd suggest enable that for now and see what users think, if a lot of users don't like we could make calva disable it by default, but I think only by enabling it as default we would catch feedback, for this feature at least.

@PEZ
Copy link
Collaborator

PEZ commented Oct 17, 2021

Thinking a bit more about it. We display evaluation results in the hover, so it can't be just a choice wether it is the REPL powered hover or clojure-lsp's one. I'm not sure what the middleware allows.

@PEZ
Copy link
Collaborator

PEZ commented Oct 18, 2021

And now having checked a bit at the code for the hover, I think the evaluation results would be safe even if we used lsp hovers instead of the Calva native ones. VS Code treats it as different hover ”slots” or whatever I should call them. So that's all good then. All options are still available. 😄

@bpringe
Copy link
Member

bpringe commented Oct 22, 2021

@ericdallo That's great! Is there a way to add a bit of padding at the top of each each example?

image

@ericdallo
Copy link
Contributor

@bpringe clojure-lsp already return new lines between then, it just seem vscode ignores it :)
From emacs and vim for example, it looks good

PEZ added a commit that referenced this issue Oct 24, 2021
Adding a command for testing how to get clojuredocs.
(Which can't be done yet, see:
clojure-lsp/clojure-lsp#598)
Addressing #689
PEZ added a commit that referenced this issue Oct 24, 2021
PEZ added a commit that referenced this issue Oct 27, 2021
PEZ added a commit that referenced this issue Oct 31, 2021
Adding a command for testing how to get clojuredocs.
(Which can't be done yet, see:
clojure-lsp/clojure-lsp#598)
Addressing #689
PEZ added a commit that referenced this issue Oct 31, 2021
PEZ added a commit that referenced this issue Oct 31, 2021
Adding a command for testing how to get clojuredocs.
(Which can't be done yet, see:
clojure-lsp/clojure-lsp#598)
Addressing #689
@PEZ PEZ linked a pull request Oct 31, 2021 that will close this issue
14 tasks
PEZ added a commit that referenced this issue Nov 1, 2021
PEZ added a commit that referenced this issue Nov 1, 2021
Adding a command for testing how to get clojuredocs.
(Which can't be done yet, see:
clojure-lsp/clojure-lsp#598)
Addressing #689
@bpringe bpringe closed this as completed in 1f65f15 Nov 1, 2021
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 a pull request may close this issue.

7 participants