clojure-emacs / orchard

A fertile ground for Clojure tooling
Eclipse Public License 1.0
323 stars 54 forks source link

[inspector] Rework pagination and datafy rendering #253

Closed alexander-yakushev closed 2 months ago

alexander-yakushev commented 2 months ago

Problem

Pagination operations apply only to the primary object being inspected. You can't prev/next page large metadata or datafied representation. With datafied, you actually can, but only by luck – if the primary object is pageable, then the datafied representation will also page along. If metadata or datafied value are larger than page size, but the primary value is not, then you only get one page of meta and datafied and no way to jump to next page or full representation.

There are a few bugs around pagination where pressing Space on a non-paged object will raise an exception.

ARef inspection (atoms, for example) also reuses the paginated rendering but doesn't respond to prev/next page.

Solution

Rework pagination. Pagination data (whether object is pageable, whether it should be paged, chunk to be displayed, last-page) is computed once and early, rendering commands reuse this information. Pagination commands only apply if the inspected value is a collection or array.

Rework datafy. This is a bit tricky because the problem is tricky. Two different cases here:

  1. (datafy inspected-value) returns something different from the value (so, there is a meaningful datafy implementation for it). Regardless of what is returned, we treat it as detached from the structure of inspected-value. So, it doesn't follow pagination rules of the inspected value. If it is small enough (< page-size), then render it completely, otherwise render it as a single clickable value that the user can navigate to and have a full-featured inspector for it.

  2. The inspected value is a map or some sequence. We know that we will paginate it, and currently display only one page (chunk). If none of the items displayed on the page datafy into something meaningful, then don't display datafy section at all. Otherwise, render datafied section. As user pages through the collection, the datafied section follows along.

If meta section is < page-size, then render it as a colelction in full, otherwise render it as a single value.

If atom contents is a collection < page-size, then render it as a map in full, otherwise render it as a single value. Don't render datafy section for atoms (we basically do the same manually in the Deref section, but with one less click for the user).

vemv commented 2 months ago

Looks very much reasonable to me.

I'm not sure I follow though as for the pagination logic for all cases.

Would you be able to share screenshots for the logic you describe?

alexander-yakushev commented 2 months ago

Meta doesn't follow current page. Small meta is rendered in full: (with-meta (repeat :data) (zipmap (range 5) (range)))

image

Large meta is rendered as value: (with-meta (repeat :data) (zipmap (range 20) (range)))

image

If datafy returns something on the value, it doesn't follow paging. Small is rendered in full: (with-meta (repeat :data) {'clojure.core.protocols/datafy (fn [_] (repeat 5 :datafy))})

image

Large is abbreved: (with-meta (repeat :data) {'clojure.core.protocols/datafy (fn [_] (repeat 20 :datafy))})

image

If not the whole collection is datafiable, but some of the collection elements, track the current page and show datafy section if there are any datafied element on current page. (assoc (vec (repeat 20 :data)) 15 (with-meta (reify Cloneable) {'clojure.core.protocols/datafy (fn [_] :datafy)})). No datafy on first page:

image

Datafy on second page:

image

alexander-yakushev commented 2 months ago

To me, the whole dance with datafying and then datafying the elements seem a bit like an overkill – I'm not sure what real-world usecases this intends to cover, but I made the rework maximally faithful to the current master behavior.

vemv commented 2 months ago

Thanks much - I could follow.

I like the proposed behavior and it seems pretty optimal to me.

alexander-yakushev commented 2 months ago

Maybe, the whole "datafy elements of the collection" should go away, and we'll instead optionally datafy the value representation (in orchard.print). So, basically, showing datafied elements inline in the primary collection, not in a separate section. But this needs more thought, while the current PR retains the current behavior meanwhile.

alexander-yakushev commented 2 months ago

I don't think it should be called more often. If anything, it would be called twice fewer times because before we performed nav on all keys twice – in render-datafy? and render-datafy-content. Do I miss something?

vemv commented 2 months ago

You're right, as of master we also call nav on each collection sub-item.

(as an aside: I've always sensed that the Inspector is non-standard here - too eager. Being lazier would enable datafy/nav -based workflows for arbitrary user intents)

alexander-yakushev commented 2 months ago

Well, it will be lazier now in the sense that only one page of data will be datafy/naved.

Otherwise, again, I'm not much of a datafy user, so I have no idea what the usecases of aggressive "datafication" are. Haven't seen many third-party datafy implementors in the wild.

vemv commented 2 months ago

Well, it will be lazier now in the sense that only one page of data will be datafy/naved.

👍

Otherwise, again, I'm not much of a datafy user, so I have no idea what the usecases of aggressive "datafication" are. Haven't seen many third-party datafy implementors in the wild.

An existing example is https://github.com/seancorfield/next-jdbc/blob/f03b1ba3168168d1cad2580a42fcf4b604310c49/doc/datafy-nav-and-schema.md - the navigable objects close over the connection

bbatsov commented 2 months ago

Looks good to me as well.

On Mon, Apr 29, 2024, at 2:07 PM, vemv wrote:

Well, it will be lazier now in the sense that only one page of data will be datafy/naved.

👍

Otherwise, again, I'm not much of a datafy user, so I have no idea what the usecases of aggressive "datafication" are. Haven't seen many third-party datafy implementors in the wild.

An existing example is https://github.com/seancorfield/next-jdbc/blob/f03b1ba3168168d1cad2580a42fcf4b604310c49/doc/datafy-nav-and-schema.md - the navigable objects close over the connection

— Reply to this email directly, view it on GitHub https://github.com/clojure-emacs/orchard/pull/253#issuecomment-2082557683, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZLSTS3NSN7SC3F64D2ULY7YZXJAVCNFSM6AAAAABG6ANGROVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBSGU2TONRYGM. You are receiving this because you are subscribed to this thread.Message ID: @.***>