Re: [PATCH 3] New: auto display inline images under subtree when `org-cycle'.

[Ihor Radchenko]

"Christopher M. Miles" <numbchild@gmail.com> writes:

> Ihor, My new patch still have some issue in the 'fold state which I
> added "FIXME" marker. Hope can get some solution from you.

I will comment only on the first patch for now.

>      "EXPORT_OPTIONS" "EXPORT_TEXT" "EXPORT_FILE_NAME"
>      "EXPORT_TITLE" "EXPORT_AUTHOR" "EXPORT_DATE" "UNNUMBERED"
>      "ORDERED" "NOBLOCKING" "COOKIE_DATA" "LOG_INTO_DRAWER" "REPEAT_TO_STATE"
> -    "CLOCK_MODELINE_TOTAL" "STYLE" "HTML_CONTAINER_CLASS")
> +    "CLOCK_MODELINE_TOTAL" "STYLE" "HTML_CONTAINER_CLASS"
> +    "ORG-IMAGE-ACTUAL-WIDTH")

This change should also be mentioned in the commit message.

> @@ -16164,14 +16165,19 @@ buffer boundaries with possible narrowing."
>                                 (org-element-property :begin par)
>                               (re-search-forward attr-re par-end t)))
>                (match-string 1)))
> +           ;; support subtree-level property "ORG-IMAGE-ACTUAL-WIDTH" 
> specified width.
> +           (subtree-property-width
> +            (ignore-errors (org-property-or-variable-value 
> 'ORG-IMAGE-ACTUAL-WIDTH)))

You are checking the property too late. I suggest to let-bind
org-image-actual-width around the whole `cond' instead.

That way, subtree-local value of ORG-IMAGE-ACTUAL-WIDTH will have an
effect of setting `org-image-actual-width' value.

Also, is there any particular reason why you wrap
`org-property-or-variable-value' into `ignore-errors'?

Finally, you can simply do

  (org-property-or-variable-value 'org-image-actual-width) 

It will return the property value _or_ org-image-actual-width value.

> -              (car org-image-actual-width))
> +              ;; Try to use subtree-level width before fallback.
> +              (or subtree-property-width
> +                  ;; Fallback to `org-image-actual-width' if no interprable 
> width is given.
> +                  (car org-image-actual-width)))

With my last suggestion, this code will not be needed.

Also, this change should be documented in etc/NEWS and also in
"12.7 Images" section of the manual.

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92