vially / wayland-explorer

Easily browse and read Wayland protocols documentation
https://wayland.app/protocols/
MIT License
193 stars 22 forks source link

Add XML value line trimming to all XML parsers #88

Closed bbb651 closed 1 month ago

bbb651 commented 1 month ago

This fixes lists not working in the MR viewer, e.g. in http://wayland.app/protocols/wayland-protocols/333#xdg_toplevel:request:unset_fullscreen

On an unrelated note, I just discovered URL fragments don't currently work in the MR viewer

bbb651 commented 1 month ago

Turns out the type definition in fast-xml-parser were wrong in 4.0 which is the version used currently, for some reason Zed points me to local deno type definitions which are for 4.4. This commit fixed the type and this following commit allows for null in the JSDoc. 4.1 version should include the fix, can we upgrade it?

bbb651 commented 1 month ago

Actually how is fast-xml-parser used in src/gitlab-api/index.ts while only being in devDependencies and not dependencies? Afaict the code can run in the browser, because the fetch to the freedesktop gitlab api does.

bbb651 commented 1 month ago

Seems to work fine. Unrelated to this PR, maybe we should change "uncut gems" in the homepage to link to https://wayland.app/protocols/wayland-protocols? Or even display them in a more prominent way than that, it's currently close to impossible to discover being at the bottom of the sidebar

bbb651 commented 1 month ago

On an unrelated-er note, I discovered dmabuf uses - rather than * for lists, and has nested lists:

      For the 'create_immed' request,
      - on success, the server immediately imports the added dmabufs to
        create a wl_buffer. No event is sent from the server in this case.
      - on failure, the server can choose to either:
        - terminate the client by raising a fatal error.
        - mark the wl_buffer as failed, and send a 'failed' event to the
          client. If the client uses a failed wl_buffer as an argument to any
          request, the behaviour is compositor implementation-defined.

image

Actually this means trimming per line doesn't work anymore and instead we need to detect the indentation, so I guess that makes it related :)

vially commented 1 month ago

Seems to work fine. Unrelated to this PR, maybe we should change "uncut gems" in the homepage to link to https://wayland.app/protocols/wayland-protocols? Or even display them in a more prominent way than that, it's currently close to impossible to discover being at the bottom of the sidebar

Sure, changing that link sounds good to me.

I discovered dmabuf uses - rather than * for lists, and has nested lists

Yeah, I suspect more of these differences will pop-up. It would be nice if they more consistent upstream or, even better, if something else other than plain-text was being used (e.g.: markdown?).

In any case, I see two main options for improving this:

bbb651 commented 1 month ago

I started to work on it I made a simple dedent implementation as a prerequisite for nested lists (also made a discussion about it in fast-xml-parser although I don't have much hope for it, even if everything goes well it'll take some time for it to get to a release), and of course protocols are inconsistent about spaces vs tabs as well (and even mixed indentation within the same protocol, although I haven't seen a case where it's mixed within a single tag value). I think it's mostly working, but if it turns out too fragile maybe adding a dependency like dedent is worth it. I'll start on the list parsing tomorrow

bbb651 commented 1 month ago

xdg-output has mixed indentation 😑

In general I find the "indentation with 2-spaces, except descriptions that happen to be 8 spaces so let's turn them into 8-width tabs" style horrible, it renders incorrectly in most places because 4-width tabs are the default, and no one else uses this style... I feel like agreeing on a style and enforcing it in CI would be nice.

Edit: Made an upstream issue for it, although we have to deal with it for MR and old protocols anyways

bbb651 commented 4 weeks ago

Add it to the list image I'm going to end up making a pretty competent markdown implementation by the end of it 😂