emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
@ 2019-02-11 22:30 Carlos Pita
  2019-02-11 22:33 ` Carlos Pita
  2019-02-11 22:51 ` Carlos Pita
  0 siblings, 2 replies; 17+ messages in thread
From: Carlos Pita @ 2019-02-11 22:30 UTC (permalink / raw)
  To: emacs-orgmode


According to the docstring:

```
(defun org-toggle-latex-fragment (&optional arg)
  "Preview the LaTeX fragment at point, or all locally or globally.

If the cursor is on a LaTeX fragment, create the image and overlay
it over the source code, if there is none.  Remove it otherwise.
**If there is no fragment at point, display all fragments in the
current section**.
```

Notice the last sentence. Nevertheless C-c C-x C-l acts like a toggle
that will remove all previews as far as there is at least one preview
active, that is a toggle with a preference to UNdisplay all fragments in
the current section. Now, there is C-u C-u C-c C-x C-l for that. AFAICS,
the current situation is such that there is no way to get a full preview
of an entire section when at least one formula is already previewed,
except for unpreviewing and repreviewing everything. This is not only
inconvenient but also contradicts the docstring and mostly overlaps the
C-u C-u case.

I suggest to make C-c C-x C-l a toggle with preference for previewing,
that is: preview everything except when everything is already previewed.


Regards
--
Carlos

---

Emacs  : GNU Emacs 26.1.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.5)
 of 2019-02-10
Package: Org mode version 9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-11 22:30 Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)] Carlos Pita
@ 2019-02-11 22:33 ` Carlos Pita
  2019-02-11 22:51 ` Carlos Pita
  1 sibling, 0 replies; 17+ messages in thread
From: Carlos Pita @ 2019-02-11 22:33 UTC (permalink / raw)
  To: emacs-orgmode

> the current section. Now, there is C-u C-u C-c C-x C-l for that. AFAICS,

Sorry I meant just one C-u.

C-u C-u is to clear the entire buffer of previews.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-11 22:30 Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)] Carlos Pita
  2019-02-11 22:33 ` Carlos Pita
@ 2019-02-11 22:51 ` Carlos Pita
  2019-02-11 23:15   ` Carlos Pita
  1 sibling, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-11 22:51 UTC (permalink / raw)
  To: emacs-orgmode

> I suggest to make C-c C-x C-l a toggle with preference for previewing,
> that is: preview everything except when everything is already previewed.

Mmmm I think this is not easy since it would imply changing
org-remove-latex-fragment-image-overlay return value:

         (if (org-remove-latex-fragment-image-overlays beg end)
         (progn
           (message "LaTeX fragment images removed from section")
           (throw 'exit nil))

One workaround is calling the toggle twice in order to force full
preview (in case the initial state contains at least one preview).
Since everything is cached the overhead is negligible or at least
tolerable.

Another option could be to add an optional 'force argument to the
toggle to simplify user bindings to a "force preview" key.

What do you think?

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-11 22:51 ` Carlos Pita
@ 2019-02-11 23:15   ` Carlos Pita
  2019-02-12 21:45     ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-11 23:15 UTC (permalink / raw)
  To: emacs-orgmode

A last suggestion. Incidentally the toggle returns nil when at least a
fragment is unpreviewed and non-nil otherwise (as a side effect of
message). This can be documented and made part of the interface, so
that something like the following can be put together by the end user:

(defun my-org-preview-latex (&optional arg)
  (interactive "P")
  (unless (org-toggle-latex-fragment arg)
    (org-toggle-latex-fragment arg)))
I insist on the importance of the use case "force preview" because
it's very useful to quickly jot down a paragraph or part of a
paragraph with many simple latex fragments like $x_i$, $f(x)$, etc.
and then, afterwards, type C-c C-x C-l to preview the last entered
part of the paragraph, instead of painfully entering C-c C-x C-l after
any single $x$ or similar. Now, if the toggle prefers removing
previews as it's the case, this won't work because, in general, you
already have some preview in the same section or paragraph.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-11 23:15   ` Carlos Pita
@ 2019-02-12 21:45     ` Nicolas Goaziou
  2019-02-12 22:00       ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-12 21:45 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Hello,

Carlos Pita <carlosjosepita@gmail.com> writes:

> A last suggestion. Incidentally the toggle returns nil when at least a
> fragment is unpreviewed and non-nil otherwise (as a side effect of
> message). This can be documented and made part of the interface, so
> that something like the following can be put together by the end user:
>
> (defun my-org-preview-latex (&optional arg)
>   (interactive "P")
>   (unless (org-toggle-latex-fragment arg)
>     (org-toggle-latex-fragment arg)))
> I insist on the importance of the use case "force preview" because
> it's very useful to quickly jot down a paragraph or part of a
> paragraph with many simple latex fragments like $x_i$, $f(x)$, etc.
> and then, afterwards, type C-c C-x C-l to preview the last entered
> part of the paragraph, instead of painfully entering C-c C-x C-l after
> any single $x$ or similar. Now, if the toggle prefers removing
> previews as it's the case, this won't work because, in general, you
> already have some preview in the same section or paragraph.

Here is my take on the issue.

First, I think the current behaviour is as it is because deleting
overlays is fast, whereas checking if some LaTeX fragments are not
previewed is slow. So, there is little overhead to first delete them: if
that is what you wanted, you win, if you wanted to preview more
fragments, you only need to hit `C-c C-x C-l` a second time. OTOH, if we
do the opposite, as you suggest, you gain one `C-c C-x C-l' -- or any
shorter binding -- in the latter case, but can waste a lot of time in
the former case, i.e., quitting preview is not fast anymore.

In any case, I agree the current behavior is slightly confusing. At
least, we could clean the overlapping between `C-u C-c C-x C-l' and `C-c
C-x C-l'. For example :

  - C-c C-x C-l :: Toggle preview on the fragment at point, *raise an
    error outside a fragment*
  - C-u C-c C-x C-l :: Toggle preview for current section
  - C-u C-u C-c C-x C-l :: Toggle preview for the whole document

Or, if we consider that previewing the whole document is not very useful
and can be given a more cumbersome binding,

  - C-c C-x C-l :: Toggle preview on the fragment at point, raise an
    error outside a fragment
  - C-u C-c C-x C-l :: *Preview* for current section, incrementally
    (what you suggest)
  - C-u C-u C-c C-x C-l :: *Clear preview* from the whole section
  - C-u 0 C-u C-c C-x C-l :: *Preview* the whole document
  - C-u - C-c C-x C-l :: *Clear preview* for the whole document

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-12 21:45     ` Nicolas Goaziou
@ 2019-02-12 22:00       ` Carlos Pita
  2019-02-12 22:43         ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-12 22:00 UTC (permalink / raw)
  To: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

What about leaving everything as it is now and adding C-c C-x C-S-l to mean
"force preview", of course with the C-u and C-u C-u variants. This is a bit
more orthogonal in the sense that the numerical argument controls scope and
the S modifier controls "forcing". Also, it's backwards compatible with
established muscle memory. What do you think? I could write a patch.

[-- Attachment #2: Type: text/html, Size: 416 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-12 22:00       ` Carlos Pita
@ 2019-02-12 22:43         ` Nicolas Goaziou
  2019-02-12 23:23           ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-12 22:43 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Carlos Pita <carlosjosepita@gmail.com> writes:

> What about leaving everything as it is now and adding C-c C-x C-S-l to mean
> "force preview", of course with the C-u and C-u C-u variants. This is a bit
> more orthogonal in the sense that the numerical argument controls scope and
> the S modifier controls "forcing". Also, it's backwards compatible with
> established muscle memory. What do you think? I could write a patch.

`C-c C-x C-S-l` is too ugly, even for me. It is a convention we don't
use in Org.

This doesn't solve the overlapping between `C-c C-x C-l' and `C-u C-x
C-l' either.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-12 22:43         ` Nicolas Goaziou
@ 2019-02-12 23:23           ` Carlos Pita
  2019-02-13 14:25             ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-12 23:23 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

> `C-c C-x C-S-l` is too ugly, even for me. It is a convention we don't
> use in Org.

Mmm ok :).

I proposed it because it is easy to remember if you think you're
modifying a base action by S and also because it's easier to keep C
pressed (versus simply S-l or M-l).

So lets play with minus as a modifier, I like that idea.

(A) Here is a variation of my proposal:

[C- -] [C-u] [C-u] C-c C-x C-l

The modifier [C- -] means force preview.
The modifier [C-u] means section scope.
The modifier [C-u][C-u] means document scope.

So - means force, C-u means section, C-u C-u means document.

One advantage of this approach is backwards compatibility.

(B) Here is a variation of your proposal. In it - means clear (I find
this a good mnemonic since "minus removes stuff"):

- C-c C-x C-l :: Toggle preview on the fragment at point, raise an
    error outside a fragment
- C-u C-c C-x C-l :: *Preview* for current section
- C-- C-u C-c C-x C-l :: *Clear preview* from the current section
- C-u C-u C-c C-x C-l :: *Preview* the whole document
- C-- C-u C-u C-c C-x C-l :: *Clear preview* for the whole document

So - means clear, C-u means section, C-u C-u means document.

> This doesn't solve the overlapping between `C-c C-x C-l' and `C-u C-x
> C-l' either.

I know I mentioned this overlapping but that was the result of a
confusion of mine: at first I mistakenly thought the C-u modifiers
were there to force preview clearing. But I don't think the
section-scope overlapping between C-u C-c C-x C-l and C-c C-x C-l when
used outside a fragment is a such bad thing. The C-u modifiers can be
thought as setting "strict scopes" of operation while the vanilla
operation tries to be smart. The problem with this smartness is not
the overlapping per se but that the meaning of "toggle" is ill defined
when you have a mixed set of un/previewed fragments. Therefore,
although I'm ok with the section-scope overlapping, I agree that it
could be convenient to ban the toggling behavior altogether except for
single fragments, for which it's well defined. But backwards
compatibility is a balancing consideration.

I'm fine with both (A) and (B) above. (A) is backwards compatible and
(B) removes the somewhat surprising toggle behavior when outside a
fragment (which motivated this report).

Best regards
--
Carlos

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-12 23:23           ` Carlos Pita
@ 2019-02-13 14:25             ` Nicolas Goaziou
  2019-02-13 14:53               ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-13 14:25 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Hello,

Carlos Pita <carlosjosepita@gmail.com> writes:

> So lets play with minus as a modifier, I like that idea.
>
> (A) Here is a variation of my proposal:
>
> [C- -] [C-u] [C-u] C-c C-x C-l
>
> The modifier [C- -] means force preview.
> The modifier [C-u] means section scope.
> The modifier [C-u][C-u] means document scope.
>
> So - means force, C-u means section, C-u C-u means document.
>
> One advantage of this approach is backwards compatibility.
>
> (B) Here is a variation of your proposal. In it - means clear (I find
> this a good mnemonic since "minus removes stuff"):
>
> - C-c C-x C-l :: Toggle preview on the fragment at point, raise an
>     error outside a fragment
> - C-u C-c C-x C-l :: *Preview* for current section
> - C-- C-u C-c C-x C-l :: *Clear preview* from the current section
> - C-u C-u C-c C-x C-l :: *Preview* the whole document
> - C-- C-u C-u C-c C-x C-l :: *Clear preview* for the whole document
>
> So - means clear, C-u means section, C-u C-u means document.

This is daunting. 

I have another, simpler, suggestion. First, we can drop document scope
altogether. We may still provide a command for it, but binding it seems
not useful. Also, if we keep the "smart" behaviour of `C-c C-x C-l', we
can also get rid of section scope. This leaves plenty of space to
distinguish between previewing and clearing previews.

Concretely

- `C-c C-x C-l' :: When on a LaTeX fragment, toggle previewing, as
  usual. Outside of LaTeX fragment, preview the whole section,
  unconditionally. In particular, if there is nothing (more) to preview,
  do nothing.

- `C-u C-c C-x C-l' :: Clear all previews in the current section,
  unconditionally.

There is no overlap, clearing previews is still fast, and you can
preview LaTeX fragments incrementally.

If we absolutely need to bind document preview, we can still keep it
bound in `C-u 0 C-c C-x C-l' (`C-u - C-c C-x C-l' to clear), but it
doesn't need a binding out of the box.

WDYT?

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 14:25             ` Nicolas Goaziou
@ 2019-02-13 14:53               ` Carlos Pita
  2019-02-13 15:10                 ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-13 14:53 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

> WDYT?

I like it. Indeed, I was tempted to suggest removing document scope
but, as an end user, I moderate my proposals to be more or less
conservative.

There are some complications though. If we remove the document scope
bindings we have to refactor the current function quite a bit, because
the interface it provides is purely interactive relying on numerical
arguments. Maybe a split would be in order. I don't like this aspect
that much.

What do you think of this variation of your last proposal:

C-c C-x C-l: as you defined it
C-u C-c C-x C-l: preview document scope.
C-- (or C-0) C-c C-x C-l: as you defined C-u C-c C-x C-l.
C-- (or C-0) C-u C-c C-x C-l: unpreview document scope.

Here I'm keeping both of your bindings although C-u is changed to C--
(or C-0, I think both are good mnemonics - = remove, 0 = leave zero).
Then C-u is free to be used to signal document scope. I dislike the
idea of swapping the roles of this modifiers because of the mnemonic
advantage, even if "clearing previews" is to be used more often than
"document scope".

If you prefer to keep just your two bindings instead, we need to
discuss how to offer the "document scope" interface to the end user.

Best regards
--
Carlos

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 14:53               ` Carlos Pita
@ 2019-02-13 15:10                 ` Carlos Pita
  2019-02-13 16:24                   ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-13 15:10 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

> C-c C-x C-l: as you defined it
> C-u C-c C-x C-l: preview document scope.
> C-- (or C-0) C-c C-x C-l: as you defined C-u C-c C-x C-l.
> C-- (or C-0) C-u C-c C-x C-l: unpreview document scope.

Btw, I don't think that "preview the entire document" is such a rare
use case. Consider that you've taken some quick notes using embedded
latex (I need to do that often because my notes are almost exclusively
about mathematical stuff and unicode is far from enough). Now you open
the notes and you can i. export to pdf and preview using
docview/pdf-tools/external pdf reader or, alternatively, ii. preview
all fragments. I agree it's more usual to go to some section of
interest and then preview it, but nevertheless full preview has its
place.

This proposal makes more cumbersome to unpreview the entire document,
which I do think is barely necessary. But the other use cases are just
one modifier away from the base use case (toggle fragment). The
downside is that C-0 is assigned to an arguably less frequent use case
than C-u, because of the mnemonic argument. As I said, I dislike
swapping them, but if you prefer it that way I'm fine with it; in that
case what will result is your proposal plus two C-0 or something
variations for full document.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 15:10                 ` Carlos Pita
@ 2019-02-13 16:24                   ` Nicolas Goaziou
  2019-02-13 16:43                     ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-13 16:24 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Carlos Pita <carlosjosepita@gmail.com> writes:

>> C-c C-x C-l: as you defined it
>> C-u C-c C-x C-l: preview document scope.
>> C-- (or C-0) C-c C-x C-l: as you defined C-u C-c C-x C-l.
>> C-- (or C-0) C-u C-c C-x C-l: unpreview document scope.
>
> Btw, I don't think that "preview the entire document" is such a rare
> use case. Consider that you've taken some quick notes using embedded
> latex (I need to do that often because my notes are almost exclusively
> about mathematical stuff and unicode is far from enough). Now you open
> the notes and you can i. export to pdf and preview using
> docview/pdf-tools/external pdf reader or, alternatively, ii. preview
> all fragments. I agree it's more usual to go to some section of
> interest and then preview it, but nevertheless full preview has its
> place.

I don't want to remove the possibility to preview a full document.
However, I prefer not binding it instead of binding it to an awfully
long key sequence. For example, if the function responsible for
previewing the full document is called `org-latex-preview-all', I'm sure
I can fire `M-x org-latex-preview-all RET' at least as fast as `C-0 C-u
C-c C-x C-l'. And if I need it often enough, I could bind it to, e.g.,
`C-c v' and be done with it.

> This proposal makes more cumbersome to unpreview the entire document,
> which I do think is barely necessary. But the other use cases are just
> one modifier away from the base use case (toggle fragment). The
> downside is that C-0 is assigned to an arguably less frequent use case
> than C-u, because of the mnemonic argument. As I said, I dislike
> swapping them, but if you prefer it that way I'm fine with it; in that
> case what will result is your proposal plus two C-0 or something
> variations for full document.

Even if C-0 or C-- are good mnemonics, C-u is, in addition to being
easier to type, the universal argument. For any given binding `B', one
can reasonably expect to find the most usual alternative action bound to
`C-u B'. Here, clearing previewing is much more useful than previewing
the whole document. As a user, I would rather expect it under `C-u C-c
C-x C-l`.

On the implementation side, all previewing functions are just a wrapper
away from `org-format-latex'. For example:

    (defun org--latex-preview-region (beg end)
      "Preview LaTeX fragments between BEG and END."
      (let ((file (buffer-file-name (buffer-base-buffer))))
        (org-format-latex
         (concat org-preview-latex-image-directory "org-ltximg")
         beg end
         ;; Emacs cannot overlay images from remote hosts.  Create
         ;; it in `temporary-file-directory' instead.
         (if (or (not file) (file-remote-p file))
    	 temporary-file-directory
           default-directory)
         'overlays nil 'forbuffer org-preview-latex-default-process)))

    (defun org-latex-preview-all (&optional arg)
      "Preview all LaTeX fragments throughout the document.
    When optional argument ARG is non-nil, remove previews instead."
      (if arg (org-remove-latex-fragment-image-overlays)
        (org--latex-preview-region (point-min) (point-max))))

We can then re-use `org--latex-preview-region' for previewing a section,
or only the LaTeX fragment at point.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 16:24                   ` Nicolas Goaziou
@ 2019-02-13 16:43                     ` Carlos Pita
  2019-02-13 19:38                       ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-13 16:43 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

Ok, let's make this more concrete so I can start working on it then.

Alternative A:

Provide three functions:

org-toggle-latex-fragment:
    bound to C-c C-x C-l
    has an optional argument arg
    delegates to org-preview-latex-section if necessary (outside of
fragment or C-u)

org-preview-latex-section:
    unbound
    has an optional argument remove

org-preview-latex-all:
    unbound
    has an optional argument remove

----

Alternative B:

Do some cosmetic changes to org-toggle-latex-fragment and provide the
following bindings:

C-c C-x C-l: toggle fragment or preview section
C-u C-c C-x C-l: unpreview section
C-u C-u C-c C-x C-l: preview document
C-u C-u C-u C-c C-x C-l: unpreview document
I favor alternative B since it's a simpler change and the only awful
binding is the (by far) less useful one.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 16:43                     ` Carlos Pita
@ 2019-02-13 19:38                       ` Carlos Pita
  2019-02-14  0:23                         ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-13 19:38 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

Nicolas, here is a patch implementing alternative B above with
ORG-NEWS entry and everything.

I have been playing with it and find the bindings quite handy.

Code is indeed a bit simpler.

If you like it, feel free to amend it before merging.

Best regards
--
Carlos

[-- Attachment #2: 0001-org-Make-latex-preview-toggle-more-useful-and-predic.patch --]
[-- Type: text/x-patch, Size: 4587 bytes --]

From 799ecd332e81a31b06f69ba5546db74eb9583ba7 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Wed, 13 Feb 2019 16:26:46 -0300
Subject: [PATCH] org: Make latex preview toggle more useful and predictable

* lisp/org.el (org-toggle-latex-fragment):
  - Avoid toggling behavior for subtree/buffer scope
  - Make common use cases simpler to type

* Detailed discussion:

  http://lists.gnu.org/archive/html/emacs-orgmode/2019-02/msg00138.html
---
 lisp/org.el | 71 ++++++++++++++++++++++++++---------------------------
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 003058434..afd3f8709 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18057,60 +18057,59 @@ overlays were removed, nil otherwise."
     overlays))
 
 (defun org-toggle-latex-fragment (&optional arg)
-  "Preview the LaTeX fragment at point, or all locally or globally.
-
-If the cursor is on a LaTeX fragment, create the image and overlay
-it over the source code, if there is none.  Remove it otherwise.
-If there is no fragment at point, display all fragments in the
-current section.
-
-With prefix ARG, preview or clear image for all fragments in the
-current subtree or in the whole buffer when used before the first
-headline.  With a prefix ARG `\\[universal-argument] \
-\\[universal-argument]' preview or clear images
-for all fragments in the buffer."
+  "Toggle preview of the LaTeX fragment at point.
+
+If the cursor is on a LaTeX fragment, create the image and
+overlay it over the source code, if there is none.  Remove it
+otherwise.
+
+If there is no fragment at point, display image for all fragments
+in the current section.
+
+With prefix ARG, clear image for all fragments in the current
+subtree.
+
+With double prefix ARG, display image for all fragments in the
+buffer.
+
+With triple prefix ARG, clear image for all fragments in the
+buffer."
   (interactive "P")
   (when (display-graphic-p)
     (catch 'exit
       (save-excursion
 	(let (beg end msg)
 	  (cond
-	   ((or (equal arg '(16))
-		(and (equal arg '(4))
-		     (org-with-limited-levels (org-before-first-heading-p))))
-	    (if (org-remove-latex-fragment-image-overlays)
-		(progn (message "LaTeX fragments images removed from buffer")
-		       (throw 'exit nil))
-	      (setq msg "Creating images for buffer...")))
-	   ((equal arg '(4))
+	   ((member arg '((16) (64)))	; Double or triple prefix
+	    (when (org-remove-latex-fragment-image-overlays)
+	      (message "LaTeX fragments images removed from buffer"))
+	    (when (equal arg '(64)) (throw 'exit nil))
+	    (setq msg "Creating images for buffer..."))
+	   ((member arg '((4)))		; Single prefix
 	    (org-with-limited-levels (org-back-to-heading t))
 	    (setq beg (point))
 	    (setq end (progn (org-end-of-subtree t) (point)))
-	    (if (org-remove-latex-fragment-image-overlays beg end)
-		(progn
-		  (message "LaTeX fragment images removed from subtree")
-		  (throw 'exit nil))
-	      (setq msg "Creating images for subtree...")))
+	    (when (org-remove-latex-fragment-image-overlays beg end)
+	      (message "LaTeX fragment images removed from subtree"))
+	    (throw 'exit nil))
 	   ((let ((datum (org-element-context)))
 	      (when (memq (org-element-type datum)
 			  '(latex-environment latex-fragment))
 		(setq beg (org-element-property :begin datum))
 		(setq end (org-element-property :end datum))
-		(if (org-remove-latex-fragment-image-overlays beg end)
-		    (progn (message "LaTeX fragment image removed")
-			   (throw 'exit nil))
-		  (setq msg "Creating image...")))))
+		(when (org-remove-latex-fragment-image-overlays beg end)
+		  (message "LaTeX fragment image removed")
+		  (throw 'exit nil))
+		(setq msg "Creating image..."))))
 	   (t
 	    (org-with-limited-levels
 	     (setq beg (if (org-at-heading-p) (line-beginning-position)
 			 (outline-previous-heading)
-			 (point)))
-	     (setq end (progn (outline-next-heading) (point)))
-	     (if (org-remove-latex-fragment-image-overlays beg end)
-		 (progn
-		   (message "LaTeX fragment images removed from section")
-		   (throw 'exit nil))
-	       (setq msg "Creating images for section...")))))
+			 (point))
+		   end (progn (outline-next-heading) (point)))
+	     (when (org-remove-latex-fragment-image-overlays beg end)
+	       (message "LaTeX fragment images removed from section"))
+	       (setq msg "Creating images for section..."))))
 	  (let ((file (buffer-file-name (buffer-base-buffer))))
 	    (org-format-latex
 	     (concat org-preview-latex-image-directory "org-ltximg")
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-13 19:38                       ` Carlos Pita
@ 2019-02-14  0:23                         ` Nicolas Goaziou
  2019-02-14  1:31                           ` Carlos Pita
  0 siblings, 1 reply; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-14  0:23 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Carlos Pita <carlosjosepita@gmail.com> writes:

> Nicolas, here is a patch implementing alternative B above with
> ORG-NEWS entry and everything.
>
> I have been playing with it and find the bindings quite handy.
>
> Code is indeed a bit simpler.
>
> If you like it, feel free to amend it before merging.

Thank you.

I eventually merged both alternatives, refactored some code and renamed
a few functions.

Please let me know if anything went wrong.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-14  0:23                         ` Nicolas Goaziou
@ 2019-02-14  1:31                           ` Carlos Pita
  2019-02-14 14:52                             ` Nicolas Goaziou
  0 siblings, 1 reply; 17+ messages in thread
From: Carlos Pita @ 2019-02-14  1:31 UTC (permalink / raw)
  To: Carlos Pita, emacs-orgmode

> Please let me know if anything went wrong.

I think your refactor improves the original code a lot and makes clear
that toggling is just a special case.

I've been testing the changes with a pretty complex beamer document
and found no fault.

Thanks!

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)]
  2019-02-14  1:31                           ` Carlos Pita
@ 2019-02-14 14:52                             ` Nicolas Goaziou
  0 siblings, 0 replies; 17+ messages in thread
From: Nicolas Goaziou @ 2019-02-14 14:52 UTC (permalink / raw)
  To: Carlos Pita; +Cc: emacs-orgmode

Hello,

Carlos Pita <carlosjosepita@gmail.com> writes:

> I think your refactor improves the original code a lot and makes clear
> that toggling is just a special case.
>
> I've been testing the changes with a pretty complex beamer document
> and found no fault.

Great! Thank you for the feedback and the suggested implementation!

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-02-14 14:52 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 22:30 Bug: org-toggle-latex-fragment doesn't work as documented [9.2.1 (release_9.2.1-60-gb0379f @ /home/carlos/local/stow/emacs/share/emacs/site-lisp/org/)] Carlos Pita
2019-02-11 22:33 ` Carlos Pita
2019-02-11 22:51 ` Carlos Pita
2019-02-11 23:15   ` Carlos Pita
2019-02-12 21:45     ` Nicolas Goaziou
2019-02-12 22:00       ` Carlos Pita
2019-02-12 22:43         ` Nicolas Goaziou
2019-02-12 23:23           ` Carlos Pita
2019-02-13 14:25             ` Nicolas Goaziou
2019-02-13 14:53               ` Carlos Pita
2019-02-13 15:10                 ` Carlos Pita
2019-02-13 16:24                   ` Nicolas Goaziou
2019-02-13 16:43                     ` Carlos Pita
2019-02-13 19:38                       ` Carlos Pita
2019-02-14  0:23                         ` Nicolas Goaziou
2019-02-14  1:31                           ` Carlos Pita
2019-02-14 14:52                             ` Nicolas Goaziou

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).