Hello Ikumi, On 16/10/2025, Ikumi Keita wrote: > Sorry for further delay, I've been laid up with a terrible cold since > Sunday night, and I've finally managed to get back to reviewing. No worries, I hope you are feeling better!
> Your proposal basically looks good to me and I think we can accept it. > There is one minor concern: why do you treat dvisvgm as if it were a > variant of dvipng? It seems more intuitive to introduce > `preview-dvisvgm-process-setup', `preview-start-dvisvgm' etc. The point is that the dvipng functions are exactly the same as the dvisvgm functions, at least in the refactored code where everything is controlled with `preview-dvipng-image-type` and `preview-dvipng-command`. I agree that dvisvgm being a variant of dvipng is weird, that is why I proposed in the second patch I attached to rename all symbols to `preview-dvi*-process-setup`, `preview-start-dvi*` etc so that they include both dvipng and dvisvgm as two customization of a common interface (the obsolete aliases would ensure code is still backward compatible). The other option, as you said, is to introduce new functions which call helper functions with TYPE and COMMAND arguments fixed to those compatible with dvipng or dvisvgm. This is what I did first (I can probably recover it from my preview-point branch), but I found it to be too cumbersome as I had to pass TYPE and COMMAND from function to functions to sentinels (and had to change the interface of existing functions to accept those arguments). It also required some code duplication. Overall, it felt more natural to treat the dvi conversion as the key feature, rather than the output file which can be customized. Let me know if you disagree, and what you think about the second path renaming functions. > -(defun preview-dvipng-process-setup () > - "Set up dvipng process for conversion." > +(defun preview-dvi*-process-setup-1 (img-type start sentinel) > + "Setup process of a DVI converter to IMG-TYPE. > +START is the function that actually starts the process and SENTINEL is > +the used `TeX-sentinel-function'." > > The last sentence seems to lack a word between "the" and "used" (and > possibly "by" is also missing?) It's my awkward phrasing. Perhaps simply: "START is the function that actually starts the process with `TeX-sentinel-function' set to SENTINEL." -- Al _______________________________________________ bug-auctex mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-auctex
