emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@posteo.net>
To: Karthik Chikmagalur <karthikchikmagalur@gmail.com>
Cc: stardiviner <numbchild@gmail.com>, Org mode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH v8] Inline image display as part of a new org-link-preview system
Date: Mon, 28 Oct 2024 18:16:53 +0000	[thread overview]
Message-ID: <875xpcks8a.fsf@localhost> (raw)
In-Reply-To: <87h68weovw.fsf@gmail.com>

Karthik Chikmagalur <karthikchikmagalur@gmail.com> writes:

> 1. Patch 0001 is the code patch from before, with the warnings from make
> addressed.
>
> 2. Patch 0002 is the documentation patch, which adds to ORG-NEWS and the
> manual.  There are two new sections -- one is in the appendix under
> "Hacking", and shows how to add a preview function for a link type
> (along with a code example).

Thanks!
See my comments on the documentation patch below.

> -** Images
> +** Images and link previews
> +
> +Org mode can preview hyperlinks in the buffer as images or with other
> +decorations.

May put a link to [*Hyperlinks] here.

Also, this reads awkward. Maybe something like

    Org mode can display previews of [[*Hyperlinks][hyperlinks]] inside
    Org buffers.  By default, only image links[fn::previews are
    available =file:= and =attachment:= links to image files; Emacs
    should be able to display the linked images (see ~image-types~
    variable)] can be previewed - the images are displayed in place of
    the link path.

    You can customize the previews as described in [*Adding Hyperlink
    preview] section.  The custom previews do not have to display images
    - any kind of [[info:elisp#Overlay Properties][display decoration]]
    can be used.

> +*** External link previews
> +:PROPERTIES:
> +:DESCRIPTION: Preview links in the buffer.
> +:END:
> +
> +Org mode can preview [[* External links][External links]] as inline
> +images or with other decorations.  This requires support for the
> +corresponding link types to be available[fn:: To add support for or to
> +modify how links of a certain type are previewed, see .].  By default
                                                     ^^^^^
> +Org mode includes support for previewing file and attachment links for
> +image files (see [[* Images][Images]]).

The first sentence in the parent section and the first sentence here are
basically identical. As I suggested above, let's move these things
up. (Feel free to modify my version as you see fit.)

> +Supported link types can be previewed within the buffer with the
> +following commands:

If possible, try to use active voice. We are talking to users in the
manual:

   You can preview the supported link types in the whole buffer, in
   current section, region, or at point.

> +  By default, only image links without a description are displayed.
> +  You can force displaying previews for all supported links using
> +  numeric argument to toggle all the images in current section (~1~
> +  argument) or the whole buffer (~11~ argument).

This is not accurate. C-1 argument will act just as no argument: display
links at point/region/section depending on the context.

Looks like the docstring wrt C-1/C-11 args should be improved.

> +  #+vindex: org-startup-with-inline-images
> +  You can ask for inline link previews to be displayed at startup by
> +  configuring the variable ~org-startup-with-inline-images~[fn:: The
> +  variable ~org-startup-with-inline-images~ can be set within a buffer
> +  with the =STARTUP= options =inlineimages= and =noinlineimages=.].

     Org mode can display link previews automatically when opening
     buffers.  Either customize ~org-startup-with-inline-images~ or use
     in-buffer =#+STARTUP: inlineimages= keyword to enable the previews.
     =#+STARTUP: noinlineimages= will disable the previews in specific
     buffer.

Also, since we are generalizing things away from images, maybe we should
introduce some aliases to ~org-startup-with-inline-images~ and to the
startup option values. Something like ~org-startup-with-link-previews~
and similar to startup option.

> +- {{{kbd(C-c C-x C-M-v)}}} (~org-link-preview-refresh~) ::
> +
> +  #+kindex: C-c C-x C-M-v
> +  #+findex: org-link-preview-refresh
> +  Assure inline display of external link previews in the buffer and
> +  refresh them.

"the whole buffer" maybe.

> +Such images can be displayed within the buffer with the
> +~org-link-preview~ and associated command, see [[*External link previews]].

This sentence reads awkwardly after the previous section that already
talked about previews and ~org-link-preview~ command.

We can write

  When [[*External link previews][link previews]] are displayed as
  images, the image size and alignment can be further customized.

>  #+vindex: org-cycle-inline-images-display
>  Inline images can also be displayed when cycling the folding state.
> @@ -21794,6 +21833,65 @@ ** Adding Hyperlink Types
>     topic.  It also provides a default description.  The function
>     ~org-insert-link~ can insert it back into an Org buffer later on.

I suspect that it belongs to the previous section - cycling should work
for all the link previews, not just images.
  
> +2. The value of the =:preview= parameter must be a function that
> +   accepts three arguments:
> +   - an overlay placed from the start to the end of the link,
> +   - the link path, as a string, and
> +   - the link element.

element -> syntax node

> +   It must return a non-nil value to indicate preview success.

... and your example can sometimes return nil, which is confusing.
We should explain what nil means as a return value as well.

> --- a/etc/ORG-NEWS
> +++ b/etc/ORG-NEWS
> @@ -24,26 +24,38 @@ Previously, =C-c C-x C-v= always toggled image display in the whole
>  buffer (or narrowed part of the buffer).  With prefix argument, it
>  also forced displaying image links with description.
> ...
> +2. Otherwise, if there is an image at point, it is toggled.  If there
> +   is no image at point, images in the current entry are previewed

Aside: I am wondering if we should also attach preview toggle to <TAB>
when ~org-cycle-inline-images-display~ is non-nil and there is a link at
point.

> +*** New option ~org-link-preview-batch-size~
> +
> +Org link previews are generated a few at a time, in batches.  This

asynchronously

> +option controls the number of links that are previewed in each batch.
> +
> +*** New option ~org-link-preview-delay~
> +
> +Org link previews are generated asynchronously.  This option controls
> +the minimum idle time in seconds between previews of batches of links.

-- 
Ihor Radchenko // yantar92,
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>


  reply	other threads:[~2024-10-28 18:16 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-15  3:28 [PATCH] add a function to only refresh inline images under current headline instead of global buffer Christopher M. Miles
2023-05-15 11:08 ` Ihor Radchenko
2023-05-15 13:01   ` Christopher M. Miles
2023-05-15 14:00   ` [PATCH v2] " Christopher M. Miles
2023-05-16  9:17     ` Ihor Radchenko
2023-05-16 12:18       ` Christopher M. Miles
2023-07-24 11:25         ` Ihor Radchenko
2023-08-01  4:40           ` [PATCH v3] " Christopher M. Miles
2023-08-01  8:04             ` Ihor Radchenko
2023-08-01 12:17               ` [PATCH v3.1] " Christopher M. Miles
2023-08-01 14:09                 ` Ihor Radchenko
2023-08-01 15:22                   ` Christopher M. Miles
2023-08-01 15:46                   ` Christopher M. Miles
2023-08-02 16:08                     ` Ihor Radchenko
2023-08-04  6:30                       ` Christopher M. Miles
2023-08-02  7:26                 ` Ihor Radchenko
2023-08-02 15:44                   ` Christopher M. Miles
2023-08-04  8:20                     ` Ihor Radchenko
2023-08-05  5:28                       ` [PATCH v3.2] " Christopher M. Miles
2024-07-22 10:46                         ` Ihor Radchenko
2024-08-01 22:58                           ` [PATCH v4.0] " stardiviner
     [not found]                           ` <66a8b73b.170a0220.383476.996e@mx.google.com>
2024-08-12 10:18                             ` Ihor Radchenko
2024-08-14  2:04                               ` stardiviner
2024-08-18 10:27                                 ` Ihor Radchenko
2024-08-20  2:02                                   ` Karthik Chikmagalur
2024-08-20 15:43                                     ` Karthik Chikmagalur
2024-08-20 18:19                                       ` Ihor Radchenko
2024-08-20 19:46                                         ` Karthik Chikmagalur
2024-08-22 13:19                                           ` Ihor Radchenko
2024-08-23  6:04                                             ` Karthik Chikmagalur
2024-08-23 23:36                                             ` [PATCH v1] Inline image display as part of a new org-link-preview system Karthik Chikmagalur
2024-08-24  1:00                                               ` Karthik Chikmagalur
2024-08-31 14:22                                               ` Ihor Radchenko
2024-08-31 16:41                                                 ` Karthik Chikmagalur
2024-08-31 16:53                                                   ` Ihor Radchenko
2024-08-31 22:37                                                     ` [PATCH v2] " Karthik Chikmagalur
2024-09-01 13:06                                                       ` Ihor Radchenko
2024-09-02 20:13                                                         ` [PATCH v3] " Karthik Chikmagalur
2024-09-08  7:43                                                           ` Ihor Radchenko
2024-09-09  3:21                                                             ` Karthik Chikmagalur
2024-09-09  6:06                                                               ` Ihor Radchenko
2024-09-09  6:30                                                                 ` Karthik Chikmagalur
2024-09-09 16:47                                                                   ` Ihor Radchenko
2024-09-09 19:14                                                                     ` Karthik Chikmagalur
2024-09-10 16:57                                                                       ` Ihor Radchenko
2024-09-10 19:53                                                                         ` Karthik Chikmagalur
2024-09-15  7:51                                                                           ` Ihor Radchenko
2024-09-09 21:45                                                                 ` Karthik Chikmagalur
2024-09-10 16:58                                                                   ` Ihor Radchenko
2024-09-10 17:38                                                                     ` Karthik Chikmagalur
2024-09-10 18:34                                                                       ` Ihor Radchenko
2024-09-10 19:43                                                             ` Karthik Chikmagalur
2024-09-15  8:12                                                               ` Ihor Radchenko
2024-09-15 20:50                                                                 ` Karthik Chikmagalur
2024-09-15 21:57                                                                 ` [PATCH v4] " Karthik Chikmagalur
2024-09-17 18:16                                                                   ` Ihor Radchenko
2024-09-18  1:44                                                                     ` [PATCH v5] " Karthik Chikmagalur
2024-09-21 10:11                                                                       ` Ihor Radchenko
2024-09-22 22:00                                                                         ` [PATCH v6] " Karthik Chikmagalur
2024-10-03 17:03                                                                           ` Ihor Radchenko
2024-10-08  0:07                                                                             ` [PATCH v7] " Karthik Chikmagalur
2024-10-10 17:15                                                                               ` Ihor Radchenko
2024-10-10 21:23                                                                                 ` Karthik Chikmagalur
2024-10-12  8:15                                                                                   ` Ihor Radchenko
2024-10-28  6:13                                                                                     ` [PATCH v8] " Karthik Chikmagalur
2024-10-28 18:16                                                                                       ` Ihor Radchenko [this message]
2024-10-28 21:53                                                                                         ` [PATCH v9] " Karthik Chikmagalur
2024-10-29 18:46                                                                                           ` Ihor Radchenko
2024-10-29 20:45                                                                                             ` [PATCH v10] " Karthik Chikmagalur
2024-10-30 17:32                                                                                               ` Ihor Radchenko
2024-10-30 19:50                                                                                                 ` Karthik Chikmagalur
2024-08-18 10:34                                 ` [FR] Automatically display images in resutls of evaluation (was: [PATCH v4.0] Re: [PATCH] add a function to only refresh inline images under current headline instead of global buffer) Ihor Radchenko
     [not found]                                   ` <66c54dfc.a70a0220.3c823a.2899@mx.google.com>
2024-08-22 13:06                                     ` [FR] Automatically display images in resutls of evaluation Ihor Radchenko
     [not found]                                       ` <66c89411.170a0220.3255c1.0cd5@mx.google.com>
     [not found]                                         ` <87zfor7b04.fsf@localhost>
     [not found]                                           ` <CAL1eYuLOsaS43ahueN4uWiCn+Ykp=p_-t9dzAypKdy1en_53BQ@mail.gmail.com>
2024-09-15 13:33                                             ` [PATCH v3] " Ihor Radchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=875xpcks8a.fsf@localhost \
    --to=yantar92@posteo.net \
    --cc=emacs-orgmode@gnu.org \
    --cc=karthikchikmagalur@gmail.com \
    --cc=numbchild@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).