[சனி ஜூன் 15, 2024] Ihor Radchenko wrote:
> Visuwesh <visuwe...@gmail.com> writes:
>
>> Sorry for the noise, I copied the wrong link in the commit message.
>> Please see attached instead.
>
> Thanks!
> I have some comments.
>
>> - (type (assoc (plist-get params :plot-type)
>> - org-plot/preset-plot-types))
>> + (type (cdr (assoc (plist-get params :plot-type)
>> + org-plot/preset-plot-types)))
>> gnuplot-script)
>
> This may break the existing customization.
> Later in the function, TYPE is used as an argument for
> `org-plot/gnuplot-term-extra' and `org-plot/gnuplot-script-preamble'.
> Some users may have these two custom options adjusted to the older
> calling convention.
>
> To not break things, we should pass the full `assoc' to these functions.
If you meant org-plot/gnuplot-script eventually calling these functions,
then I don't see how the above change will break things. Even in
org-plot/gnuplot-script, TYPE passed to both these user options are
(let* ((type-name (plist-get params :plot-type))
(type (cdr (assoc type-name org-plot/preset-plot-types))))
so there should be no harm done by the above change since TYPE is not an
argument taken by org-plot/gnuplot-script.
> Also, while you are at it, may your please clarify what TYPE means in
> the docstrings of `org-plot/gnuplot-term-extra' and
> `org-plot/gnuplot-script-preamble'?
OK.
>> (setf params (org-plot/collect-options params))))
>> + (setq params (org-combine-plists type params))
>
> May you also drop a short comment in the code that explains what this
> line does?
I will send an updated patch once my confusion is cleared.
