[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Fix regex for determining image width from attribute
From: |
Timothy |
Subject: |
Re: [PATCH] Fix regex for determining image width from attribute |
Date: |
Wed, 01 Dec 2021 12:54:27 +0800 |
User-agent: |
mu4e 1.6.9; emacs 28.0.50 |
Hi Matt,
Thanks for your thoughtful deliberation on this.
> I think the essential disagreement is whether org should take an action if not
> explicitly told to do so. I think org should only perform some action if given
> a clear directive. In this context, I feel that org is guessing what the user
> wants and taking an action based on that guess.
I broadly agree with this, but I think this is provided by the four forms that
`org-image-actual-width' can take:
⁃ `t', in which case the actual image width is always used
⁃ an integer, in which case that will always be used as the width
⁃ `nil', which produces the guessing behaviour we’re discussing
⁃ `(val)', which guesses, falling back on `val'
> Ok, back to the fact that there are multiple considerations here. The
> first issue is whether specifying a width for a backend reflects an
> intention to have that same width in the org buffer. As I previously
> stated, I don’t agree that one implies the other. But, as also
> previously discussed, this was a decision that was made almost 10 years
> ago, so changing it would be a breaking change, etc. Because of that,
> I’m not totally sure what org should do, and I expect a lot of people
> won’t want to change this.
I’m not opposed to /expanding/ the behaviour (with due consideration), which
could
resolve some of your concerns, but I don’t think it would be good to prevent the
current behaviour, which at this point seems well-established.
> The other consideration is if we take the first point as a given (that
> org should use width directives for other backends), should it also
> attempt to interpret directives that are ambiguous? In this case, do we
> interpret 1.2 as 1.2? If could only be ,
> I’d be inclined to agree that this is logical. I also understand the
> case for , though this is slightly less clear. But, what if
> someone used 1.2? Seems a bit unusual I know, but maybe
> someone would want this. Again, I don’t think we should guess if there’s
> a chance we could be wrong.
I feel this very much depends on how bad “guessing wrong” is, and as previously
discussed, since it’s rather easy to correct or set `org-image-actual-width' to
prevent this, I’m not sure it warrants being terribly concerned about.
> I totally agree with you that we don’t want to implement a pseudo latex
> parser here. But I feel like all this complexity is easily resolved by
> just requiring that people be explicit about their intentions (i.e.,
> specify #+attr_org: :width). That would avoid all the complex behavior
> and surprises that could result from making intelligent guesses about
> what the user wants.
I think prioritising `#+attr_org: :width' makes a lot of sense, but I feel quite
reluctant to /require/ it.
> Anyway, let me know what you want in terms of the patch. I still think
> prioritizing attr_org should be its own patch and changing the regex and
> all the other behavior should be a separate issue. But, if you’d like me
> to perform the change I mentioned in my last email, I can take the time
> to write that up and include it in the same patch.
Thanks for continuing with this. Moving forward, I think it would be best to:
⁃ Make a patch just for prioritising `#+attr_org'
⁃ Make a patch just improving the regex (before or after the `#+attr_org' patch)
⁃ Discuss changing the behaviour of image previews separately later / in another
thread, linking to this thread when doing so.
How does that sound?
Lastly, a comment on your documentation patch from earlier. I like the changes
to `org-image-actual-width', however I think you’ve been over-eager with
scrapping
the current docstring for `org-display-inline-image--width'. Since the behaviour
is implemented there, I think it should at a minimum be documented there.
The docstring for a function referring to a variable’s documentation for how
it’s
handled by the function seems a bit weird.
All the best,
Timothy
- Re: [PATCH] Fix regex for determining image width from attribute,
Timothy <=