clojure-emacs / cider

The Clojure Interactive Development Environment that Rocks for Emacs
https://cider.mx
GNU General Public License v3.0
3.52k stars 643 forks source link

Add visual indentation of sections in ClojureDocs buffer #3705

Closed katomuso closed 4 weeks ago

katomuso commented 1 month ago

Replace * with - for elements of the list in "See Also" section as most users are more familiar with - used for lists.

Remove = used before headings as this markup feels redundant because of the following change.

Add visual indentation of sections to improve contrast between section heading and content. This indentation is purely visual because when copying an example or note, it may be inconvenient if the indentation prefix was added before them. Also, maybe we should make it possible for users to change the indentation prefix.

This is how it looked before: clojuredocs-indentation-1

This is how it looks now: clojuredocs-indentation-2

bbatsov commented 1 month ago

I think the change from * to - is quite subjective, but more importantly - it's important to make sure that the UI in this buffer is reasonably consistent with the regular docs buffers, that can also display See Also. Perhaps it won't be bad to add some button there to see the ClojureDocs or go one step further and just embed the remaining data from ClojureDocs (mostly examples) in the regular docs, so we'd reduce the need to use ClojureDocs standalone.

katomuso commented 1 month ago

Perhaps it won't be bad to add some button there to see the ClojureDocs or go one step further and just embed the remaining data from ClojureDocs (mostly examples) in the regular docs, so we'd reduce the need to use ClojureDocs standalone.

I think it's a good idea to make it possible to show data from ClojureDocs in regular doc buffer so there will be one user interface for this particular need. We can make it possible to show examples from ClojureDocs by default, but I think it will be a good idea to make it customizable as some users may find it distracting. I will look into ways we can do this.

katomuso commented 1 month ago

What do you think specifically about the visual indentation feature? I think it can improve readability for most users (it was inspired by the popular org-indent-mode), and the indentation prefix can easily be customizable, making it possible to disable indentation if needed.

bbatsov commented 1 month ago

I'm OK with it, but I'm not sure it improves the readability much.

katomuso commented 1 month ago

I'm OK with it, but I'm not sure it improves the readability much.

Maybe, but it looks neat for sure :)

katomuso commented 1 month ago

As I see more and more usage of the nrepl-dict.el package, I'm curious why we construct a special dictionary and use the nrepl-dict.el package specifically for working with these dictionaries instead of utilizing the functions provided by map.el? To me, they seem similar to property lists, just prefixed with dict, but perhaps there's something I'm missing.

katomuso commented 1 month ago

As I see more and more usage of the nrepl-dict.el package, I'm curious why we construct a special dictionary and use the nrepl-dict.el package specifically for working with these dictionaries instead of utilizing the functions provided by map.el? To me, they seem similar to property lists, just prefixed with dict, but perhaps there's something I'm missing.

Actually, the commentary for nrepl-dict.el says that they are indeed simply plists with an extra element at the head. I think that it may be a good idea to replace it with the built-in map.el at some point in time. What do you think?

bbatsov commented 1 month ago

map.el didn't exist back then. Changing the dict should be done with extreme caution, as every package depending on CIDER is using it and breaking changes are not an option. Frankly, I wouldn't bother with this, unless there would be some meaningful gains from the change.

katomuso commented 1 month ago

map.el didn't exist back then. Changing the dict should be done with extreme caution, as every package depending on CIDER is using it and breaking changes are not an option. Frankly, I wouldn't bother with this, unless there would be some meaningful gains from the change.

Okay, got it.

katomuso commented 1 month ago

I tried to make sense of the cider-doc.el codebase, but it seems that it requires too much additional work to clean it up. Since the implementation of cider-clojuredocs.el is much simpler, I think it's better to keep working on it for now. In the future, we can incorporate good ideas from it into cider-doc.el to create a more unified interface.

Can we merge this PR, or do you think the changes in markup and visual indentation are too opinionated and not particularly useful, so we should skip them?

bbatsov commented 1 month ago

I'll think about the next steps here over the weekend. Everything in CIDER has a lot of history and even I forget it from time to time. I took me a while to remember why the buffers look the way they did. Basically, originally we were using Grimoire instead of ClojureDocs (as Grimoire had a handy API I could use and ClojureDocs only had some daily data exports that I needed to process) and we wanted to have the same formatting as there and possibly render the markdown somehow as HTML (see https://github.com/clojure-emacs/cider/pull/1959). I guess I adapted the old code hastily to ClojureDocs when I made the switch. (as Grimoire got shut down)

I've been thinking now that too many docs buffers are probably a mistake as many people probably have no idea about the ClojureDocs integration, that's why I suggested it might be easier to just add the examples in the regular docs buffers (the "see alsos" info is already there) and dispense with this separate view completely. I'm not particularly opposed to the changes you've made (except for my attachment to * I guess), but I'm also wondering if there's any point in improving this if only a handful of people are using it.

//cc @vemv @alexander-yakushev

alexander-yakushev commented 1 month ago

I never use Clojuredocs integration. I have C-c C-d C-d muscle-memorized so this is what I will use 99% of the time without thinking. I also like the rendering of regular cider-doc buffer over cider-clojuredocs, it has better formatting and shows extra information extracted from metadata.

Given that the only real benefit of Clojuredocs is the examples, I think it would make plenty of sense to merge those into cider-doc. That way, more people will get benefit of seeing them.

vemv commented 1 month ago

Personally I'd keep doc and clojuredocs decoupled but linked, e.g. have a See ClojureDocs examples button at the bottom.

When querying doc, most times I only need a quick 'refresher' and seeing a bunch of examples would feel visually overwhelming.

katomuso commented 1 month ago

Thanks for the feedback! So I guess I have no choice but to start working on cider-doc.el.

yuhan0 commented 1 month ago

Just for reference, here's a small tweak I made locally to add [Inspect Value] and [View Clojuredocs] buttons to a regular cider-doc buffer for easy navigation: https://github.com/clojure-emacs/cider/commit/b2b61b6b4e63457d08be64ee8ead72fdcbf69f5b

Given that the only real benefit of Clojuredocs is the examples, I think it would make plenty of sense to merge those into cider-doc. That way, more people will get benefit of seeing them.

This is in fact what Calva does (over at VS Code), appending them directly below the docstring: image

But I agree with @vemv that this ends up feeling a bit too noisy / cluttered, at least for my tastes. For someone less familiar with Clojure it may be helpful to have these examples always near at hand.

alexander-yakushev commented 1 month ago

Fair enough. A button/link that says "View examples" may a good middleground.

katomuso commented 1 month ago

What do you think about making a customizable variable that controls whether you want to display examples in the *clojure-doc* buffer or not?

If the value of this variable is nil, there will be a button to show examples. When the user clicks on this button, the current *clojure-doc* buffer will be redisplayed with this variable dynamically set to t, so the examples will be displayed just that one time.

This way, we can please both groups: those who want to show examples only after the button is pressed and those who want to show examples by default. Also, this way we can fully get rid of cider-clojuredocs.el, as there is actually no benefit compared to the regular doc buffer except for showing the examples (and notes) and it will be easier to maintain a single package (cider-doc.el) for this kind of feature.

vemv commented 4 weeks ago

What do you think about making a customizable variable that controls whether you want to display examples in the clojure-doc buffer or not?

I'd keep them separate (but linked) because the 'examples' category could plausibly evolve to be different.

I'm thinking, most examples tend to be runnable, so they could be rendered in a way that makes them super easy to be run, or even edited.

Even without that, cider-doc essentially isn't a clojure-mode buffer (it should be eventually capable of rendering Markdown, etc), while 'examples' sounds like something that is best derived from clojure-mode.

With all this said, we could render examples at the bottom, optionally, as long as that doesn't mean that we ditch a dedicated buffer or conflate misc codebase parts.

it will be easier to maintain a single package (cider-doc.el) for this kind of feature.

I actually disagree with this - smaller .el files with fewer responsibilities are simpler to mantain. They can still 'communicate' (get examples in cider-clojuredocs.el, optionally render those in cider-doc.el)

bbatsov commented 4 weeks ago

I actually disagree with this - smaller .el files with fewer responsibilities are simpler to mantain. They can still 'communicate' (get examples in cider-clojuredocs.el, optionally render those in cider-doc.el)

Normally, I'd agree, but if you check the ClojureDocs functionality you'd see it's pretty much the same as the regular docs, plus the examples. The "see also" functionality was integrated with the standard docs years ago, and there's little use for ClojureDocs as a standalone buffer, unless someone wants to get more or less what they'd get on the ClojureDocs site. (but I fail to see why would anyone care about this) Like @alexander-yakushev I don't really use this (and I have a feeling that very few people do), so I don't care much what we do here, but if we want to more people to see the examples, we'll have to change our approach. CIDER has way too many features today and only a handful of the users actually know about many of them.

I like the idea for a "Show examples" button (if there are example to show), as that's simpler than configuration and yields more or less the same result. I doubt that many people will figure out there's a config they have to set to be able to see examples in the docs.

vemv commented 4 weeks ago

I like the idea for a "Show examples" button (if there are example to show), as that's simpler than configuration

Well yeah, I don't think anyone was advocating for hiding this under a defcustom.

Show examples could, rather than merely expand a collapsed section, open a new buffer. This way the examples buffer can be in clojure-mode, and user-editable.

It would seem something very CIDER-y to do - foster evaluation of forms.

Also, examples could not only come from ClojureDocs, but also :examples metadata or other sources.

bbatsov commented 4 weeks ago

Yeah, that's fair - if we want people to actually be able to interact with the examples they can be opened in a different buffer.

katomuso commented 4 weeks ago

As @vemv pointed out, I wasn't suggesting hiding it under a defcustom. Instead, I meant that the button to display examples should always be visible. However, for those who prefer to have the examples shown by default, there should be a customizable variable to control this behavior. For users who want to see examples only after pressing the button, I proposed that the current buffer be redisplayed with the examples included, eliminating the need to switch to a separate buffer just to view examples.

Personally, I don’t see much benefit in evaluating examples since they are typically brief and straightforward, often accompanied by comment that show the result of the evaluation. I usually look at examples just to refresh some idioms in my memory, so I’m not particularly motivated to implement such a feature. However, if you want to integrate examples into the Doc buffer, I’m willing to help with that.

bbatsov commented 4 weeks ago

Personally, I don’t see much benefit in evaluating examples since they are typically brief and straightforward, often accompanied by comment that show the result of the evaluation. I usually look at examples just to refresh some idioms in my memory, so I’m not particularly motivated to implement such a feature. However, if you want to integrate examples into the Doc buffer, I’m willing to help with that.

That'd be enough for me, as that's what I had in mind when I suggested just fusing the two docs buffers. In general I don't see the need to make it possible to evaluate the examples and this can be done separately down the road if someone has the desire to do so.

katomuso commented 4 weeks ago

So it's settled then. I will close this PR as there is probably no need for it anymore, okay?

bbatsov commented 4 weeks ago

Yep. Likely at the end we'll just remove this dedicated buffer completely (after some deprecation period), as it will be redundant.