jgm / djot.js

JavaScript implementation of djot
MIT License
146 stars 16 forks source link

Respect "Source positions" checkbox #40

Closed hellux closed 1 year ago

matklad commented 1 year ago

While we are at it, maybe disable it by default? Source positions add quite a bit of noise to html/ast outputs, and, at least for me, are rarely useful.

jgm commented 1 year ago

I enabled it by default in parsing because the scroll sync of the preview to the text depends on it. At the very least, I'd like it to be enabled by default, but a few iterations ago I had it working so that the preview pane always got source positions but rendered versions didn't. That would be ideal.

hellux commented 1 year ago

I would also prefer it to be disabled by default, I have never needed to look at the source positions, personally.

The scroll sync only works in preview, right? I guess we could always enable source positions for preview because they are not visible anyway (only if inspecting the element).

hellux commented 1 year ago

Got it working I think, I got confused by the sourcePositions option to djot.renderHTML, though. It did not seem to have any effect, leftover from lua version?

Preview always has source positions now regardless of the checkbox state, and scroll sync thus always works.

Because the scroll position may change when changing mode (e.g. box is unchecked and going from preview to html -> should go from on to off), one can no longer skip parsing the AST and only render.

jgm commented 1 year ago

Great, this makes sense. Some of these were hangovers from the earlier Lua version.