Re: Proposal to fix simultaneity (Re: Onwards to lexical binding (attempt 1))

[Ikumi Keita]

> Ikumi Keita  writes:
> Tassilo Horn  writes:
>>> I expect the attached patch addresses this issue. What do you think
>>> about it?

>> Yes, looks correct and simple enough.  So I'm all in favor of it.

> Thanks, I installed the change in the git repo.

Sorry, this breaks region preview for some reason I haven't found out
yet. I'll revert it for now.

Bye,
Ikumi Keita
#StandWithUkraine #StopWarInUkraine

Re: Proposal to fix simultaneity (Re: Onwards to lexical binding (attempt 1))

[Ikumi Keita]

> Tassilo Horn  writes:
>> I expect the attached patch addresses this issue. What do you think
>> about it?

> Yes, looks correct and simple enough.  So I'm all in favor of it.

Thanks, I installed the change in the git repo.

Cheers,
Ikumi Keita
#StandWithUkraine #StopRussianAggression

Re: Proposal to fix simultaneity (Re: Onwards to lexical binding (attempt 1))

[Tassilo Horn]

Ikumi Keita  writes:

Hi Keita,

> This is a continuation of 1.5-year old thread.

Better late than never. :-)

>>> I'd say it's not serious but we should address it anyway, and making
>>> it local in the process buffer seems like a suitable approach.
>
> I expect the attached patch addresses this issue. What do you think
> about it?

Yes, looks correct and simple enough.  So I'm all in favor of it.

Bye,
Tassilo

Proposal to fix simultaneity (Re: Onwards to lexical binding (attempt 1))

[Ikumi Keita]

Hi Tassilo,

This is a continuation of 1.5-year old thread.

> Ikumi Keita  writes:
> Tassilo Horn  writes:
>> Ikumi Keita  writes:
>>> The reason is that sentinel functions `TeX-LaTeX-sentinel' and those in
>>> preview.el call `TeX-active-master', which depends on the global dynamic
>>> scope variable 'TeX-current-process-region-p'. Suppose that Alice types
>>> C-c C-c in a fairly large document.  The LaTeX process takes, say, 20
>>> seconds or more. Before it finishes, Alice spans a region in the buffer
>>> and types C-c C-r.
>>> Then 'TeX-current-process-region-p' is overwritten by that C-c C-r, thus
>>> `TeX-master-file', called in the process sentinel of the first process
>>> invoked by C-c C-c, doesn't work as expected.

>> Hm, isn't it actually even worse, i.e., Alice could compile one large
>> document and then compile/preview a region in some completely different
>> document?  AFAICS, `TeX-current-process-region-p' isn't even
>> buffer-local which would at least help with the "different documents"
>> case.

> Yes, I agree with you.

>> Well, if we've had that working at some point in time, I'd say it would
>> be good if we could restore that.

>> But how did it work?  I mean, the sentinel function runs much later
>> outside of the dynamic scope of the command starting the process.

> I guess that in early stage of development, sentinel functions were
> simple enough to be free of TeX-active-master, including implicit
> dependence through other functions.

>>> On the other hand, if this problem must be considered as a serious
>>> defect, then we should turn 'TeX-current-process-region-p' into a kind
>>> of "process local variable" like `TeX-command-next'.

>> I'd say it's not serious but we should address it anyway, and making it
>> local in the process buffer seems like a suitable approach.

I expect the attached patch addresses this issue. What do you think
about it?

Regards,
Ikumi Keita
#StandWithUkraine #StopRussianAggression

>From 6b9ae7aa6933bcb10580227fb77e6407770d0207 Mon Sep 17 00:00:00 2001
From: Ikumi Keita 
Date: Sun, 7 Nov 2021 17:23:26 +0900
Subject: [PATCH] Fix simultaneity

* tex-buf.el (TeX-command): Keep the value of
`TeX-current-process-region-p' as buffer-local value of
`TeX--this-process-region-flag' in process buffer.
(TeX-command-sentinel): Call sentinel where
`TeX-current-process-region-p' has its own value when the process
started.
(TeX-current-process-region-p): Move `defvar' before its first usage.
(TeX--this-process-region-flag): New variable.
---
 tex-buf.el | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tex-buf.el b/tex-buf.el
index eeff2fc0..f5a2d22e 100644
--- a/tex-buf.el
+++ b/tex-buf.el
@@ -285,7 +285,6 @@ at bottom if LINE is nil."
 ;; far down (i.e. further down than their first use), so we have to pre-declare
 ;; them here to explain it to the compiler.
 ;; We should move those vars's definitions earlier instead!
-(defvar TeX-current-process-region-p)
 (defvar TeX-save-query)
 (defvar TeX-parse-function)
 (defvar TeX-sentinel-function)
@@ -487,6 +486,11 @@ Do you want to select one of these engines? "
   :group 'TeX-command
   :type 'integer)
 
+(defvar TeX-current-process-region-p nil
+  "Non-nil means that the last TeX command is on a region.")
+(defvar-local TeX--this-process-region-flag nil
+  "Per process value of `TeX-current-process-region-p'.")
+
 (defun TeX-command (name file-fn  override-confirm)
   "Run command NAME on the file returned by calling FILE-FN.
 
@@ -556,7 +560,9 @@ remember to add /Library/TeX/texbin/ to your PATH"
 ;; Now start the process
 (let ((file (funcall file-fn)))
   (TeX-process-set-variable file 'TeX-command-next TeX-command-Show)
-  (funcall hook name command file
+  (funcall hook name command file)
+  (TeX-process-set-variable file 'TeX--this-process-region-flag
+TeX-current-process-region-p
 
 (defun TeX-command-expand (command  list)
   "Expand COMMAND for `TeX-active-master' as described in LIST.
@@ -1453,8 +1459,8 @@ Insert MSG with some additional information."
  (TeX-command-mode-line process)
  (setq TeX-command-next TeX-command-Show)
  (goto-char (point-min))
- (apply TeX-sentinel-function process name nil)
-
+ (let ((TeX-current-process-region-p TeX--this-process-region-flag))
+   (funcall TeX-sentinel-function process name))
 
  ;; If buffer and mode line will show that the process
  ;; is dead, we can delete it now.  Otherwise it
@@ -2028,9 +2034,6 @@ command."
 
 ;;; Active Process
 
-(defvar TeX-current-process-region-p nil
-  "Non-nil means that the last TeX command is on a region.")
-
 (defun TeX-active-process ()
   "Return the active process for the current buffer."
   (TeX-process (TeX-active-master)))
-- 
2.34.1

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

> Tassilo Horn  writes:
> Ikumi Keita  writes:
>> The reason is that sentinel functions `TeX-LaTeX-sentinel' and those in
>> preview.el call `TeX-active-master', which depends on the global dynamic
>> scope variable 'TeX-current-process-region-p'. Suppose that Alice types
>> C-c C-c in a fairly large document.  The LaTeX process takes, say, 20
>> seconds or more. Before it finishes, Alice spans a region in the buffer
>> and types C-c C-r.
>> Then 'TeX-current-process-region-p' is overwritten by that C-c C-r, thus
>> `TeX-master-file', called in the process sentinel of the first process
>> invoked by C-c C-c, doesn't work as expected.

> Hm, isn't it actually even worse, i.e., Alice could compile one large
> document and then compile/preview a region in some completely different
> document?  AFAICS, `TeX-current-process-region-p' isn't even
> buffer-local which would at least help with the "different documents"
> case.

Yes, I agree with you.

> Well, if we've had that working at some point in time, I'd say it would
> be good if we could restore that.

> But how did it work?  I mean, the sentinel function runs much later
> outside of the dynamic scope of the command starting the process.

I guess that in early stage of development, sentinel functions were
simple enough to be free of TeX-active-master, including implicit
dependence through other functions.

>> On the other hand, if this problem must be considered as a serious
>> defect, then we should turn 'TeX-current-process-region-p' into a kind
>> of "process local variable" like `TeX-command-next'.

> I'd say it's not serious but we should address it anyway, and making it
> local in the process buffer seems like a suitable approach.

Sounds reasonable. How do others think?

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

>> I think `preview-region' should bind TeX-current-process-region-p
>> permanently by `setq', not `let', so that TeX-active-master returns
>> intended file name even in process sentinels.
>
> This incident made me aware that it has been long that AUCTeX lost the
> simultaneity, under certain condition, stated in the last sentence of
> this comment of tex-buf.el:
> ,
> | ;; The general idea is, that there is one process and process buffer
> | ;; associated with each master file, and one process and process buffer
> | ;; for running TeX on a region.   Thus, if you have N master files, you
> | ;; can run N + 1 processes simultaneously.
> `
> The reason is that sentinel functions `TeX-LaTeX-sentinel' and those in
> preview.el call `TeX-active-master', which depends on the global dynamic
> scope variable 'TeX-current-process-region-p'. Suppose that Alice types
> C-c C-c in a fairly large document.  The LaTeX process takes, say, 20
> seconds or more. Before it finishes, Alice spans a region in the buffer
> and types C-c C-r.
> Then 'TeX-current-process-region-p' is overwritten by that C-c C-r, thus
> `TeX-master-file', called in the process sentinel of the first process
> invoked by C-c C-c, doesn't work as expected.

Hm, isn't it actually even worse, i.e., Alice could compile one large
document and then compile/preview a region in some completely different
document?  AFAICS, `TeX-current-process-region-p' isn't even
buffer-local which would at least help with the "different documents"
case.

> What should we do? I don't think personally that this is a serious
> problem because it isn't much likely to run the master file process
> and the region file process in parallel in actual situation of editing
> LaTeX document.  In fact, it seems that nobody claimed about this
> problem in the past.

Yes, that's also my reasoning.

> So it's only a matter of inconsistency of comment in tex-buf.el and we
> can easily update the relavant sentence, IMHO.

Well, if we've had that working at some point in time, I'd say it would
be good if we could restore that.

But how did it work?  I mean, the sentinel function runs much later
outside of the dynamic scope of the command starting the process.  E.g.,
with

--8<---cut here---start->8---
;; -*- lexical-binding: t; -*-

(defvar th/dynmamic-test-var "Default")

(defun th/test-process-sentinel ()
  (interactive)
  (let* ((p (start-process "foo" nil "sleep" "5"))
 (th/dynmamic-test-var "dyn bound value")
 (copy-of-cur-val th/dynmamic-test-var))
(set-process-sentinel p (lambda (_p _s)
  (message "th/dynmamic-test-var: %S
copy-of-cur-val: %S"
   th/dynmamic-test-var
   copy-of-cur-val)))
(message "Started process %S" p)))
--8<---cut here---end--->8---

the sentinel function prints the value "Default" for
th/dynmamic-test-var (but "dyn boun value" for the closed over lexical
var copy-of-cur-val)...

> On the other hand, if this problem must be considered as a serious
> defect, then we should turn 'TeX-current-process-region-p' into a kind
> of "process local variable" like `TeX-command-next'.

I'd say it's not serious but we should address it anyway, and making it
local in the process buffer seems like a suitable approach.

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hello Tassilo and all,

> Ikumi Keita  writes:
> It doesn't work for region preview. After Setting region on circ.tex and
> C-c C-p C-r, I get error and in process buffer I find
> Running `Preview-PDF2DSC' with ``pdf2dsc circ.pdf 
> circ.prv/tmpSYCDuG/preview.dsc''
> . Thus preview-latex provides "master" file name "circ.pdf" rather than
> "_region_.pdf" for pdf2dsc for this case.

> I think `preview-region' should bind TeX-current-process-region-p
> permanently by `setq', not `let', so that TeX-active-master returns
> intended file name even in process sentinels.

This incident made me aware that it has been long that AUCTeX lost the
simultaneity, under certain condition, stated in the last sentence of
this comment of tex-buf.el:
,
| ;; The general idea is, that there is one process and process buffer
| ;; associated with each master file, and one process and process buffer
| ;; for running TeX on a region.   Thus, if you have N master files, you
| ;; can run N + 1 processes simultaneously.
`
The reason is that sentinel functions `TeX-LaTeX-sentinel' and those in
preview.el call `TeX-active-master', which depends on the global dynamic
scope variable 'TeX-current-process-region-p'. Suppose that Alice types
C-c C-c in a fairly large document. The LaTeX process takes, say, 20
seconds or more. Before it finishes, Alice spans a region in the buffer
and types C-c C-r.
Then 'TeX-current-process-region-p' is overwritten by that C-c C-r, thus
`TeX-master-file', called in the process sentinel of the first process
invoked by C-c C-c, doesn't work as expected.

What should we do? I don't think personally that this is a serious
problem because it isn't much likely to run the master file process and
the region file process in parallel in actual situation of editing LaTeX
document. In fact, it seems that nobody claimed about this problem in
the past.

So it's only a matter of inconsistency of comment in tex-buf.el and we
can easily update the relavant sentence, IMHO.

On the other hand, if this problem must be considered as a serious
defect, then we should turn 'TeX-current-process-region-p' into a kind
of "process local variable" like `TeX-command-next'.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

> Tassilo Horn  writes:
> Very good!  I've also had the same idea just last night in order to get
> that ugly 'TeX-active-master special case out of `TeX-expand-command'.
> Please commit.

Done.:-)

Cheers,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

> I noticed a regression in TeX-command-expand.  When the file name
> contains spaces, "%o" is expanded to raw file name without suitable
> quotes for shell.
>
> How about the attached patch? It also make "%o" expansion skip the
> obtained file name. Evaluating (TeX-command-expand "%o") no longer
> falls into infinite loop even when the file name itself contains "%o".

Very good!  I've also had the same idea just last night in order to get
that ugly 'TeX-active-master special case out of `TeX-expand-command'.
Please commit.

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

I noticed a regression in TeX-command-expand. When the file name
contains spaces, "%o" is expanded to raw file name without suitable
quotes for shell.

How about the attached patch? It also make "%o" expansion skip the
obtained file name. Evaluating (TeX-command-expand "%o") no longer falls
into infinite loop even when the file name itself contains "%o".

Regards,
Ikumi Keita



0001-Make-o-compatible-again-with-file-name-with-spaces.patch.gz
Description: fix "%o" expansion

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

>> Right.  preview-generate-preview used to do that, but that seems to
>> be to late.  So in the new version, I setq
>> TeX-current-process-region-p before the calls to that and removed its
>> region-p argument.  New patch at the end of the mail.
>
> Thanks, now it functions well. It seems ready to be committed in the
> lexical-binding-attempt-1 branch.

I've done so.  Thanks for testing!

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
> Right.  preview-generate-preview used to do that, but that seems to be
> to late.  So in the new version, I setq TeX-current-process-region-p
> before the calls to that and removed its region-p argument.  New patch
> at the end of the mail.

Thanks, now it functions well. It seems ready to be committed in the
lexical-binding-attempt-1 branch.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

> Oops, by similar reason, `preview-document' should bind
> TeX-current-process-region-p to nil. Otherwise C-c C-p C-d just after
> region preview doesn't work.

Right.  preview-generate-preview used to do that, but that seems to be
to late.  So in the new version, I setq TeX-current-process-region-p
before the calls to that and removed its region-p argument.  New patch
at the end of the mail.

>> 1. My preview.el was terribly outdated.  It seems it's not
>> re-generated from preview.el.in on every "make" invocation.  I needed
>> to delete it to be re-generated...
>
> Indeed. I guess that Makefile is written in such a way...

Ah, it's only generated by ./configure.

>> 2. The first preview invocation errors out with:
>> [1{/var/lib/texmf/fonts/map/pdftex/updmap/pdftex.map}]  !name(2.tex) 
>> !offset(47)  )
>> (see the transcript file for additional
>> information)
>> Output written on _region_.pdf (1 page, 23275 bytes).
>> SyncTeX written on _region_.synctex.gz.
>> Transcript written on _region_.log.
>
>> TeX Output exited as expected with code 1 at Fri Sep 11 09:28:23
>> Running `Preview-DviPS' with ``dvips -Pwww _region_.dvi -o 
>> _region_.prv/tmpLKkFSa/preview.ps''
>> Parser: End of Preview snippet 1 unexpected
>
>> Preview-DviPS killed at Fri Sep 11 09:28:23
>> LaTeX: End of Preview snippet 1 unexpected
>> Trying again, it works and previews are displayed.  That also happens on
>> master.
>
> Hmm, strange. While pdflatex produces PDF file, preview-latex invokes
> dvips. It doesn't occur for me. Perhaps customization on preview-latex
> related options has some inconsistency.

As far as I can see, I have no preview customizations at all.  But if it
doesn't happen for you, it's probably some issue on my side.

Bye,
Tassilo

diff --git a/preview.el.in b/preview.el.in
index c14e4900..f1cbb183 100644
--- a/preview.el.in
+++ b/preview.el.in
@@ -945,9 +945,9 @@ Pure borderless black-on-white will return an empty string."
 	   (funcall (car gsfile) "dvi" t
 (file-error nil))
   (when preview-ps-file
-  (condition-case nil
-	  (preview-delete-file preview-ps-file)
-	(file-error nil)))
+(condition-case nil
+	(preview-delete-file preview-ps-file)
+  (file-error nil)))
   (setq TeX-sentinel-function nil))
 
 (defalias 'preview-dvipng-abort 'preview-dvips-abort)
@@ -3671,8 +3671,7 @@ The fourth value is the transparent border thickness."
 	 (colors (preview-dvipng-color-string preview-colors res))
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(concat (TeX-command-expand preview-dvipng-command
-		(car file))
+			(concat (TeX-command-expand preview-dvipng-command)
 " " colors resolution)
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPNG"))
@@ -3710,8 +3709,7 @@ If FAST is set, do a fast conversion."
 		(prog1
 			(TeX-command-expand (if fast
 		preview-fast-dvips-command
-	  preview-dvips-command)
-	(car file))
+	  preview-dvips-command))
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPS"))
 (setq TeX-active-tempdir tempdir)
@@ -3751,8 +3749,7 @@ If FAST is set, do a fast conversion."
 	 pdfsource
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(TeX-command-expand preview-pdf2dsc-command
-	(car file))
+			(TeX-command-expand preview-pdf2dsc-command)
 		  (setq tempdir TeX-active-tempdir
 			pdfsource (funcall (car file) "pdf" t)
 	 (name "Preview-PDF2DSC"))
@@ -3930,8 +3927,7 @@ If FORMAT-CONS is non-nil, a previous format may get reused."
 	 (master-file (expand-file-name (TeX-master-file t)))
 	 (command (preview-do-replacements
 		   (TeX-command-expand
-		(preview-string-expand preview-LaTeX-command)
-		'TeX-master-file)
+		(preview-string-expand preview-LaTeX-command))
 		   preview-dump-replacements))
 	 (preview-auto-cache-preamble nil))
 (unless (and (consp (cdr format-cons))
@@ -3952,10 +3948,8 @@ If FORMAT-CONS is non-nil, a previous format may get reused."
 \\def\\AUCTEXINPUT##1{\\catcode`/ 12\\relax\\catcode`\\ 9\\relax\\input\\detokenize{##1}\\relax}%
 \\let\\dump\\PREVIEWdump\\dump}\\input mylatex.ltx \\relax%\n" nil dump-file)
   (TeX-save-document master)
-  (prog1
-	  (preview-generate-preview
-	   nil master
-	   command)
+  (setq TeX-current-process-region-p nil)
+  (prog1 (preview-generate-preview master command)
 	(add-hook 'kill-emacs-hook #'preview-cleanout-tempfiles t)
 	(setq TeX-sentinel-function
 	  `(lambda (process string)
@@ -4003,11 +3997,11 @@ stored in `preview-dumped-alist'."
 			(save-excursion
 			  (goto-char begin)
 			  (if (bolp) 0 -1))
-  (preview-generate-preview t (TeX-region-file)
+  (setq TeX-current-process-region-p t)
+  (preview-generate-preview (TeX-region-file)
 			(preview-do-replacements
 			 (TeX-command-expand
-			  (preview-string-expand preview-LaTeX-command)
-			  'TeX-region-file)
+			  

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
>> I think `preview-region' should bind TeX-current-process-region-p
>> permanently by `setq', not `let', so that TeX-active-master returns
>> intended file name even in process sentinels.

> Indeed, below is a patch which does that.  It works for me as far as I
> have tested.  If more testing doesn't reveal any blocker, I'm slightly
> leaning towards doing that change.  IMHO, it makes the code a bit
> easier to understand.

Oops, by similar reason, `preview-document' should bind
TeX-current-process-region-p to nil. Otherwise C-c C-p C-d just after
region preview doesn't work.

> I noticed some other (unrelated) strange things:

> 1. My preview.el was terribly outdated.  It seems it's not re-generated
> from preview.el.in on every "make" invocation.  I needed to delete it to
> be re-generated...

Indeed. I guess that Makefile is written in such a way...

> 2. The first preview invocation errors out with:
> [1{/var/lib/texmf/fonts/map/pdftex/updmap/pdftex.map}]  !name(2.tex) 
> !offset(47)  )
> (see the transcript file for additional 
> information)
> Output written on _region_.pdf (1 page, 23275 bytes).
> SyncTeX written on _region_.synctex.gz.
> Transcript written on _region_.log.

> TeX Output exited as expected with code 1 at Fri Sep 11 09:28:23
> Running `Preview-DviPS' with ``dvips -Pwww _region_.dvi -o 
> _region_.prv/tmpLKkFSa/preview.ps''
> Parser: End of Preview snippet 1 unexpected

> Preview-DviPS killed at Fri Sep 11 09:28:23
> LaTeX: End of Preview snippet 1 unexpected
> Trying again, it works and previews are displayed.  That also happens on
> master.

Hmm, strange. While pdflatex produces PDF file, preview-latex invokes
dvips. It doesn't occur for me. Perhaps customization on preview-latex
related options has some inconsistency.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

 Here's the patch which was empty yesterday where I actually do
 that.
>>> 
>>> Thanks. I'll test it later, probably tommorow.
>
>> Great, thanks!
>
> It doesn't work for region preview. After Setting region on circ.tex
> and C-c C-p C-r, I get error and in process buffer I find Running
> `Preview-PDF2DSC' with ``pdf2dsc circ.pdf
> circ.prv/tmpSYCDuG/preview.dsc'' . Thus preview-latex provides
> "master" file name "circ.pdf" rather than "_region_.pdf" for pdf2dsc
> for this case.
>
> I think `preview-region' should bind TeX-current-process-region-p
> permanently by `setq', not `let', so that TeX-active-master returns
> intended file name even in process sentinels.

Indeed, below is a patch which does that.  It works for me as far as I
have tested.  If more testing doesn't reveal any blocker, I'm slightly
leaning towards doing that change.  IMHO, it makes the code a bit
easier to understand.

I noticed some other (unrelated) strange things:

1. My preview.el was terribly outdated.  It seems it's not re-generated
from preview.el.in on every "make" invocation.  I needed to delete it to
be re-generated...

2. The first preview invocation errors out with:

--8<---cut here---start->8---
[1{/var/lib/texmf/fonts/map/pdftex/updmap/pdftex.map}]  !name(2.tex) 
!offset(47)  )
(see the transcript file for additional 
information)
Output written on _region_.pdf (1 page, 23275 bytes).
SyncTeX written on _region_.synctex.gz.
Transcript written on _region_.log.

TeX Output exited as expected with code 1 at Fri Sep 11 09:28:23
Running `Preview-DviPS' with ``dvips -Pwww _region_.dvi -o 
_region_.prv/tmpLKkFSa/preview.ps''
Parser: End of Preview snippet 1 unexpected

Preview-DviPS killed at Fri Sep 11 09:28:23
LaTeX: End of Preview snippet 1 unexpected
--8<---cut here---end--->8---

Trying again, it works and previews are displayed.  That also happens on
master.

Bye,
Tassilo

diff --git a/preview.el.in b/preview.el.in
index c14e4900..6d0b9789 100644
--- a/preview.el.in
+++ b/preview.el.in
@@ -3671,8 +3671,7 @@ The fourth value is the transparent border thickness."
 	 (colors (preview-dvipng-color-string preview-colors res))
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(concat (TeX-command-expand preview-dvipng-command
-		(car file))
+			(concat (TeX-command-expand preview-dvipng-command)
 " " colors resolution)
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPNG"))
@@ -3710,8 +3709,7 @@ If FAST is set, do a fast conversion."
 		(prog1
 			(TeX-command-expand (if fast
 		preview-fast-dvips-command
-	  preview-dvips-command)
-	(car file))
+	  preview-dvips-command))
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPS"))
 (setq TeX-active-tempdir tempdir)
@@ -3751,8 +3749,7 @@ If FAST is set, do a fast conversion."
 	 pdfsource
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(TeX-command-expand preview-pdf2dsc-command
-	(car file))
+			(TeX-command-expand preview-pdf2dsc-command)
 		  (setq tempdir TeX-active-tempdir
 			pdfsource (funcall (car file) "pdf" t)
 	 (name "Preview-PDF2DSC"))
@@ -3930,8 +3927,7 @@ If FORMAT-CONS is non-nil, a previous format may get reused."
 	 (master-file (expand-file-name (TeX-master-file t)))
 	 (command (preview-do-replacements
 		   (TeX-command-expand
-		(preview-string-expand preview-LaTeX-command)
-		'TeX-master-file)
+		(preview-string-expand preview-LaTeX-command))
 		   preview-dump-replacements))
 	 (preview-auto-cache-preamble nil))
 (unless (and (consp (cdr format-cons))
@@ -4003,11 +3999,11 @@ stored in `preview-dumped-alist'."
 			(save-excursion
 			  (goto-char begin)
 			  (if (bolp) 0 -1))
+  (setq TeX-current-process-region-p t)
   (preview-generate-preview t (TeX-region-file)
 			(preview-do-replacements
 			 (TeX-command-expand
-			  (preview-string-expand preview-LaTeX-command)
-			  'TeX-region-file)
+			  (preview-string-expand preview-LaTeX-command))
 			 preview-LaTeX-command-replacements)))
 
 (defun preview-buffer ()
@@ -4046,8 +4042,7 @@ stored in `preview-dumped-alist'."
nil (TeX-master-file)
(preview-do-replacements
 (TeX-command-expand
- (preview-string-expand preview-LaTeX-command)
- 'TeX-master-file)
+ (preview-string-expand preview-LaTeX-command))
 preview-LaTeX-command-replacements)))
 
 (defun preview-environment (count)
@@ -4090,9 +4085,7 @@ It returns the started process."
   (let* ((geometry (preview-get-geometry))
 	 (commandbuff (current-buffer))
 	 (pr-file (cons
-		   (if TeX-current-process-region-p
-		   'TeX-region-file
-		 'TeX-master-file)
+		   'TeX-active-master
 		   (file-name-nondirectory file)))
 	 (master (TeX-master-file))
 	 (master-file (expand-file-name master))
diff --git 

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
>>> Here's the patch which was empty yesterday where I actually do that.
>> 
>> Thanks. I'll test it later, probably tommorow.

> Great, thanks!

It doesn't work for region preview. After Setting region on circ.tex and
C-c C-p C-r, I get error and in process buffer I find
Running `Preview-PDF2DSC' with ``pdf2dsc circ.pdf 
circ.prv/tmpSYCDuG/preview.dsc''
. Thus preview-latex provides "master" file name "circ.pdf" rather than
"_region_.pdf" for pdf2dsc for this case.

I think `preview-region' should bind TeX-current-process-region-p
permanently by `setq', not `let', so that TeX-active-master returns
intended file name even in process sentinels.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

>> Yeah, but preview-latex can bind that itself.  I mean, it has
>> separate commands for preview document and preview region and then
>> passes either TeX-master-file or TeX-region-file.  So if it binds
>> TeX-current-process-region-p itself and TeX-active-master is used
>> implicitly, it should just work.
>
> OK, I'm fine with either way, provided that other parts of AUCTeX
> which needs binding TeX-current-process-region-p, if any, are fixed as
> well.

AFAICS, except preview, everything else goes through `TeX-command' which
sets it appropriately, anyway.

>> Here's the patch which was empty yesterday where I actually do that.
>
> Thanks. I'll test it later, probably tommorow.

Great, thanks!

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
> Yeah, but preview-latex can bind that itself.  I mean, it has separate
> commands for preview document and preview region and then passes either
> TeX-master-file or TeX-region-file.  So if it binds
> TeX-current-process-region-p itself and TeX-active-master is used
> implicitly, it should just work.

OK, I'm fine with either way, provided that other parts of AUCTeX which
needs binding TeX-current-process-region-p, if any, are fixed as well.

> Here's the patch which was empty yesterday where I actually do that.

Thanks. I'll test it later, probably tommorow.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

> Understood. Preview-latex (and presumably some other parts of AUCTeX)
> calls TeX-command-expand directly, which bypasses setting
> TeX-current-process-region-p. Thus TeX-active-master doesn't work as
> expected.

Yeah, but preview-latex can bind that itself.  I mean, it has separate
commands for preview document and preview region and then passes either
TeX-master-file or TeX-region-file.  So if it binds
TeX-current-process-region-p itself and TeX-active-master is used
implicitly, it should just work.

Here's the patch which was empty yesterday where I actually do that.

diff --git a/preview.el.in b/preview.el.in
index c14e4900..b7ad0789 100644
--- a/preview.el.in
+++ b/preview.el.in
@@ -3671,8 +3671,7 @@ The fourth value is the transparent border thickness."
 	 (colors (preview-dvipng-color-string preview-colors res))
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(concat (TeX-command-expand preview-dvipng-command
-		(car file))
+			(concat (TeX-command-expand preview-dvipng-command)
 " " colors resolution)
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPNG"))
@@ -3710,8 +3709,7 @@ If FAST is set, do a fast conversion."
 		(prog1
 			(TeX-command-expand (if fast
 		preview-fast-dvips-command
-	  preview-dvips-command)
-	(car file))
+	  preview-dvips-command))
 		  (setq tempdir TeX-active-tempdir
 	 (name "Preview-DviPS"))
 (setq TeX-active-tempdir tempdir)
@@ -3751,8 +3749,7 @@ If FAST is set, do a fast conversion."
 	 pdfsource
 	 (command (with-current-buffer TeX-command-buffer
 		(prog1
-			(TeX-command-expand preview-pdf2dsc-command
-	(car file))
+			(TeX-command-expand preview-pdf2dsc-command)
 		  (setq tempdir TeX-active-tempdir
 			pdfsource (funcall (car file) "pdf" t)
 	 (name "Preview-PDF2DSC"))
@@ -3930,8 +3927,7 @@ If FORMAT-CONS is non-nil, a previous format may get reused."
 	 (master-file (expand-file-name (TeX-master-file t)))
 	 (command (preview-do-replacements
 		   (TeX-command-expand
-		(preview-string-expand preview-LaTeX-command)
-		'TeX-master-file)
+		(preview-string-expand preview-LaTeX-command))
 		   preview-dump-replacements))
 	 (preview-auto-cache-preamble nil))
 (unless (and (consp (cdr format-cons))
@@ -4003,12 +3999,12 @@ stored in `preview-dumped-alist'."
 			(save-excursion
 			  (goto-char begin)
 			  (if (bolp) 0 -1))
-  (preview-generate-preview t (TeX-region-file)
-			(preview-do-replacements
-			 (TeX-command-expand
-			  (preview-string-expand preview-LaTeX-command)
-			  'TeX-region-file)
-			 preview-LaTeX-command-replacements)))
+  (let ((TeX-current-process-region-p t))
+(preview-generate-preview t (TeX-region-file)
+			  (preview-do-replacements
+			   (TeX-command-expand
+			(preview-string-expand preview-LaTeX-command))
+			   preview-LaTeX-command-replacements
 
 (defun preview-buffer ()
   "Run preview on current buffer."
@@ -4046,8 +4042,7 @@ stored in `preview-dumped-alist'."
nil (TeX-master-file)
(preview-do-replacements
 (TeX-command-expand
- (preview-string-expand preview-LaTeX-command)
- 'TeX-master-file)
+ (preview-string-expand preview-LaTeX-command))
 preview-LaTeX-command-replacements)))
 
 (defun preview-environment (count)
@@ -4090,9 +4085,7 @@ It returns the started process."
   (let* ((geometry (preview-get-geometry))
 	 (commandbuff (current-buffer))
 	 (pr-file (cons
-		   (if TeX-current-process-region-p
-		   'TeX-region-file
-		 'TeX-master-file)
+		   'TeX-active-master
 		   (file-name-nondirectory file)))
 	 (master (TeX-master-file))
 	 (master-file (expand-file-name master))
diff --git a/tests/japanese/preview-latex.el b/tests/japanese/preview-latex.el
index 41907dfd..5ca3d968 100644
--- a/tests/japanese/preview-latex.el
+++ b/tests/japanese/preview-latex.el
@@ -228,8 +228,7 @@ String encoded in `shift_jis' can have regexp meta characters in it."
 	  (setq process (TeX-inline-preview-internal
 			 (preview-do-replacements
 			  (TeX-command-expand
-			   (preview-string-expand preview-LaTeX-command)
-			   'TeX-master-file)
+			   (preview-string-expand preview-LaTeX-command))
 			  preview-LaTeX-command-replacements)
 			 dummyfile '(nil . nil) (current-buffer)
 			 '(nil . (t . t)) dummyfile '(nil nil nil)))
@@ -263,8 +262,7 @@ String encoded in `shift_jis' can have regexp meta characters in it."
 	  (setq process (TeX-inline-preview-internal
 			 (preview-do-replacements
 			  (TeX-command-expand
-			   (preview-string-expand preview-LaTeX-command)
-			   'TeX-master-file)
+			   (preview-string-expand preview-LaTeX-command))
 			  preview-dump-replacements)
 			 dummyfile '(nil . nil) (current-buffer)
 			 nil dummyfile '(nil nil nil)))
diff --git a/tests/tex/command-expansion.el b/tests/tex/command-expansion.el
index 0de5831c..bb5c7d49 100644
--- 

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

> Ikumi Keita  writes:
> However, it seems that the first run of preview-latex on any region
> fails. It must be my conceptual patch that is guilty for that. I haven't
> looked into the actual codes yet, but my impression is that some kind of
> initialization is required to fix this...

Understood. Preview-latex (and presumably some other parts of AUCTeX)
calls TeX-command-expand directly, which bypasses setting
TeX-current-process-region-p. Thus TeX-active-master doesn't work as
expected.

>>> I think that the second argument of TeX-command-expand is no longer
>>> necessary, thanks to the fine utility TeX-active-master.

So this thought of mine wasn't correct. How about the attached patch?
It reverts TeX--master-or-region-file-with-extra-quotes to the former
form and uses the second argument of TeX-command-expand again.

Regards,
Ikumi Keita

diff --git a/tex-buf.el b/tex-buf.el
index 975c6fc9..49b53c81 100644
--- a/tex-buf.el
+++ b/tex-buf.el
@@ -551,15 +551,6 @@ without further expansion."
 			 "%"))
 		(or list (TeX-expand-list)))
 	  pat (regexp-opt (mapcar #'car list)))
-;; `TeX-command-expand' is called with `file-fn' argument being one
-;; of `TeX-master-file', `TeX-region-file' and
-;; `TeX-active-master'.  The return value of these functions
-;; sometimes needs suitable "decorations" for an argument for
-;; underlying shell or latex executable, or both, when the
-;; relavant file-fn name involves some special characters such as
-;; space and multibyte characters.  Hence embed that function in a
-;; template prepared for that purpose.
-(setq file-fn #'TeX--master-or-region-file-with-extra-quotes)
 (while (setq TeX-expand-pos (string-match pat TeX-expand-command TeX-expand-pos))
   (setq string (match-string 0 TeX-expand-command)
 	entry (assoc string list)
@@ -567,9 +558,9 @@ without further expansion."
 	arguments (cdr (cdr entry)) ;Remaining elements
 	string (save-match-data
 		 (cond
-  ((memq expansion (list 'TeX-active-master
- #'TeX-active-master))
-   (let ((res (apply file-fn arguments)))
+  ((eq expansion #'TeX-active-master)
+   (let ((res (apply #'TeX--master-or-region-file-with-extra-quotes
+   file-fn arguments)))
  ;; Advance past the file name in order to
  ;; prevent expanding any substring of it.
  (setq TeX-expand-pos
@@ -587,20 +578,29 @@ without further expansion."
 TeX-expand-command))
 
 (defun TeX--master-or-region-file-with-extra-quotes
-( extension nondirectory ask extra)
-  "Return the current master or region file name with quote for shell.
-I.e. it encloses the file name with space within quotes `\"'
-first when \" \\input\" is supplemented (indicated by dynamically
-binded variable `TeX-command-text' having string value.)  It also
-encloses the file name within \\detokenize{} when the following
-three conditions are met:
-  1. compiling with standard (pdf)LaTeX or upLaTeX
-  2. \" \\input\" is supplemented
-  3. EXTRA is non-nil (default when expanding \"%T\")
-
-Helper function of `TeX-command-expand'."
+(file-fn  extension nondirectory ask extra)
+  "Return file name with quote for shell.
+Helper function of `TeX-command-expand'.
+
+This is a kind of template.  How to use:
+Fix, by `apply-partially', the first argument FILE-FN as one of
+the three functions `TeX-master-file', `TeX-region-file' or
+`TeX-active-master'.  Then the result is just a wrapper for that
+function suitable in `TeX-command-expand'.
+
+As a wrapper described above, it passes EXTENSION, NONDIRECTORY
+and ASK to the \"bare\" function as-is, and arranges the returned
+file name for use with command shell.  I.e. it encloses the file
+name with space within quotes `\"' first when \" \\input\" is
+supplemented (indicated by dynamically binded variable
+`TeX-command-text' having string value.)  It also encloses the
+file name within \\detokenize{} when the following three
+conditions are met:
+1. compiling with standard (pdf)LaTeX or upLaTeX
+2. \" \\input\" is supplemented
+3. EXTRA is non-nil (default when expanding \"%T\")"
   (shell-quote-argument
-   (let* ((raw (funcall #'TeX-active-master extension nondirectory ask))
+   (let* ((raw (funcall file-fn extension nondirectory ask))
 	  ;; String `TeX-command-text' means that the file name is
 	  ;; given through \input command.
 	  (quote-for-space (if (and (stringp TeX-command-text)

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
> I worked upon your patch and committed that.  Should work again.
> TeX-file-fn is now gone and one should simply use `TeX-active-master' in
> TeX-expand-list{,-builtin} where formally `file' (and then
> `TeX-file-fn') has been used.

Works fine for normal typeset.

However, it seems that the first run of preview-latex on any region
fails. It must be my conceptual patch that is guilty for that. I haven't
looked into the actual codes yet, but my impression is that some kind of
initialization is required to fix this...

>> I think that the second argument of TeX-command-expand is no longer
>> necessary, thanks to the fine utility TeX-active-master.

> Indeed.  Here is an untested patch which does that (all tests still
> pass).

The attachment is empty:-)

> Since it also affects preview-latex which I did not test, I haven't
> committed it yet. If it works, preview.el.in could also be changed to
> that preview-gs-file is no cons of (TeX-active-master . file) where
> the car is unused anymore.

Agreed.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Arash Esbati]

Hi Tassilo and Keita,

Tassilo Horn  writes:

> Ikumi Keita  writes:
>
>> When "View" command is issued, TeX-command-expand calls
>> TeX-view-command-raw, which in turn calls TeX-command-expand with the
>> second argument being nil. This second call of TeX-command-expand
>> overwrites the dynamic scope variable TeX-file-fn with wrong value, so
>> the error occurs after emacs continues the execution of the first
>> TeX-command-expand.
>>
>> Attached is a conceptual patch to fix the problem.
>
> I worked upon your patch and committed that.  Should work again.
> TeX-file-fn is now gone and one should simply use `TeX-active-master' in
> TeX-expand-list{,-builtin} where formally `file' (and then
> `TeX-file-fn') has been used.

Thanks for the analysis and the fix.  "View" works again.

Best, Arash

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

Hi Chaps,

>> Debugger entered--Lisp error: (void-function nil)
>
> [...]
>
>> This happens when I set `TeX-view-program-selection' which looks like
>> this in my init file:
>
>> (setq TeX-view-program-selection '((output-pdf "SumatraPDF")
>>(output-dvi "dviout")))
>
>> When commented out, AUCTeX uses the the default `start' entry on Windows
>> which works.  Can others reproduce this?
>
> Confirmed. Similar error occurs when I turn off TeX-PDF-mode and try
> to invoke xdvi for "View" command.
>
> When "View" command is issued, TeX-command-expand calls
> TeX-view-command-raw, which in turn calls TeX-command-expand with the
> second argument being nil. This second call of TeX-command-expand
> overwrites the dynamic scope variable TeX-file-fn with wrong value, so
> the error occurs after emacs continues the execution of the first
> TeX-command-expand.
>
> Attached is a conceptual patch to fix the problem.

I worked upon your patch and committed that.  Should work again.
TeX-file-fn is now gone and one should simply use `TeX-active-master' in
TeX-expand-list{,-builtin} where formally `file' (and then
`TeX-file-fn') has been used.

> I think that the second argument of TeX-command-expand is no longer
> necessary, thanks to the fine utility TeX-active-master.

Indeed.  Here is an untested patch which does that (all tests still
pass).  Since it also affects preview-latex which I did not test, I
haven't committed it yet.  If it works, preview.el.in could also be
changed to that preview-gs-file is no cons of (TeX-active-master . file)
where the car is unused anymore.


Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Tassilo Horn  writes:

>> It's probably worth checking the failing tests, at least some of them
>> are related.
>
> I was too chickenhearted to run the tests.  I'll have a look at the
> failing ones when I find some time.  (Of course, everybody is welcomed
> to clean up after me.)

Should be fixed now.

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Arash Esbati]

Hi Tassilo,

Tassilo Horn  writes:

> Tassilo Horn  writes:
>
>> We still have such issues in tex.el with the variable `file' and some
>> others.
>
> I've done that now, too.  I still can run LaTeX and call the Evince
> viewer (TeX-evince-sync-view and TeX-pdf-tools-sync-view don't rely on
> `file' anymore but use `TeX-active-master' as suggested by Al Haji-Ali),
> so it seems I haven't broken everything [but possibly something I didn't
> test]...

Many thanks for doing this.  I agree, we should take the step towards
lexical binding.  Should all style files also go in this direction?

I can silence some other compiler warnings in tex.el with this small
patch:

diff --git a/tex.el b/tex.el
index 41e41502..c797e286 100644
--- a/tex.el
+++ b/tex.el
@@ -107,6 +107,7 @@
 ;; Others:
 (defvar tex--prettify-symbols-alist)   ; tex-mode.el
 (defvar Info-file-list-for-emacs)  ; info.el
+(defvar dbus-debug) ; dbusbind.c and dbus.el

 (defgroup TeX-file nil
   "Files used by AUCTeX."
@@ -3618,11 +3619,11 @@ Unless optional argument COMPLETE is non-nil, ``: 
'' will be appended."
(TeX-parse-argument optional (car args))
   (TeX-parse-argument optional args

-(defun TeX-arg-literal (optional  args)
+(defun TeX-arg-literal (_optional  args)
   "Insert its arguments ARGS into the buffer.
 Used for specifying extra syntax for a macro.  The compatibility
 argument OPTIONAL is ignored."
-  (apply 'insert args))
+  (apply #'insert args))


 ;;; Font Locking

I'm not sure about this warning:

In toplevel form:
tex.el:2023:1: Warning: Unused lexical argument `viewer' Disable showing
Disable logging

which applies to

(defun TeX-source-specials-view-expand-options ( viewer)
  "Return source specials command line option for viewer command.
The return value depends on the values of
`TeX-source-correlate-mode' and
`TeX-source-correlate-method-active'.  If those are nil or not
`source-specials' respectively, an empty string will be
returned."
  (if (and TeX-source-correlate-mode
   (eq (TeX-source-correlate-method-active) 'source-specials))
  (concat TeX-source-specials-view-position-flags
  (when (TeX-source-correlate-server-enabled-p)
(concat " " TeX-source-specials-view-editor-flags)))
""))

Is prefixing viewer with _ also the solution here?  I'm not familiar
with the usage of this function.

Other warnings from latex.el can be fixed easily.

Best, Arash

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

> Tassilo Horn  writes:
>> Or it can just be
>> (or TeX-exit-mark
>> (make-marker))
>> for simplicity.

> Right, just go ahead. ;-)

Done.

Bye,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Mosè Giordano  writes:

Mosè,

> thank you so much for taking the time to look into this!

Thanks for the compliments. :-)

> Running `make check` locally on this branch I get
>
> SUMMARY OF TEST RESULTS
> ---
> Files examined: 13
> Ran 42 tests, 35 results as expected, 4 unexpected, 3 skipped
> 2 files contained unexpected results:
>   tex/command-expansion.log
>   japanese/preview-latex.log
> make[1]: *** [Makefile:85: check-doit] Error 1
>
> It's probably worth checking the failing tests, at least some of them
> are related.

I was too chickenhearted to run the tests.  I'll have a look at the
failing ones when I find some time.  (Of course, everybody is welcomed
to clean up after me.)

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

>> --
>> modified   latex.el
>> @@ -1316,9 +1316,9 @@ Just like array and tabular."
>>(save-excursion
>>  (LaTeX-find-matching-begin)
>>  (end-of-line)
>> -(let ((exit-mark (if (boundp 'exit-mark)
>> - exit-mark
>> -   (make-marker
>> +(let ((TeX-exit-mark (if (boundp 'TeX-exit-mark)
>> + TeX-exit-mark
>> +   (make-marker
>>(TeX-parse-arguments args
>> --
>> Shouldn't this boundp test be markerp because TeX-exit-mark is now bound
>> always?
>
> Or it can just be
>(or TeX-exit-mark
>(make-marker))
> for simplicity.

Right, just go ahead. ;-)

Bye,
Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

> Ikumi Keita  writes:
> By the way, I think this part should be improved:
> --
> modified   latex.el
> @@ -1316,9 +1316,9 @@ Just like array and tabular."
>(save-excursion
>  (LaTeX-find-matching-begin)
>  (end-of-line)
> -(let ((exit-mark (if (boundp 'exit-mark)
> -  exit-mark
> -(make-marker
> +(let ((TeX-exit-mark (if (boundp 'TeX-exit-mark)
> +  TeX-exit-mark
> +(make-marker
>(TeX-parse-arguments args
> --
> Shouldn't this boundp test be markerp because TeX-exit-mark is now bound
> always?

Or it can just be
   (or TeX-exit-mark
   (make-marker))
for simplicity.

Bye,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Mosè Giordano]

Hi Tassilo,

thank you so much for taking the time to look into this!

Running `make check` locally on this branch I get

SUMMARY OF TEST RESULTS
---
Files examined: 13
Ran 42 tests, 35 results as expected, 4 unexpected, 3 skipped
2 files contained unexpected results:
  tex/command-expansion.log
  japanese/preview-latex.log
make[1]: *** [Makefile:85: check-doit] Error 1

It's probably worth checking the failing tests, at least some of them
are related.

Bye,
Mosè

On Fri, 4 Sep 2020 at 17:37, Tassilo Horn  wrote:
>
> Hi all,
>
> on the new branch lexical-binding-attempt-1, I've squashed all
> "reference to free variable" warnings in latex.el and context.el by
> declaring them properly with defvars and with a prefix, and changing all
> users.
>
> This affects in latex.el
>
>   LaTeX-done-mark
>   LaTeX-level
>   LaTeX-name
>   LaTeX-title
>   LaTeX-toc
>
> and in context.el
>
>   ConTeXt-done-mark
>   ConTeXt-level
>   ConTeXt-name
>   ConTeXt-title
>   ConTeXt-reference
>
> which were previously neither prefixed nor declared.
>
> Of course, that is an incompatible change which will break user
> LaTeX-section-hook functions which accessed those unprefixed variables.
> But if we want to go lexical-binding (which I think we should), there's
> no way around that.
>
> Of course, I did not test too much (and ConTeXt not at all), so it would
> be great if you could give it a whirl.  There are chances that I might
> have changed an occurrence which I shouldn't have.
>
> We still have such issues in tex.el with the variable `file' and some
> others.  If someone wants to take care of that in a similar vein, feel
> free to give it a shot.
>
> Bye,
> Tassilo
>
>
>

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
> I just did so (-> TeX-exit-mark), and also with last-optional-rejected
> (-> TeX-last-optional-rejected).  Inserting a parbox works again. :-)

Now it works for me, too, thanks!

By the way, I think this part should be improved:
--
modified   latex.el
@@ -1316,9 +1316,9 @@ Just like array and tabular."
   (save-excursion
 (LaTeX-find-matching-begin)
 (end-of-line)
-(let ((exit-mark (if (boundp 'exit-mark)
-exit-mark
-  (make-marker
+(let ((TeX-exit-mark (if (boundp 'TeX-exit-mark)
+TeX-exit-mark
+  (make-marker
   (TeX-parse-arguments args
--
Shouldn't this boundp test be markerp because TeX-exit-mark is now bound
always?

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Ikumi Keita  writes:

Hi Keita-san,

>> I've done that now, too.  I still can run LaTeX and call the Evince
>> viewer (TeX-evince-sync-view and TeX-pdf-tools-sync-view don't rely
>> on `file' anymore but use `TeX-active-master' as suggested by Al
>> Haji-Ali), so it seems I haven't broken everything [but possibly
>> something I didn't test]...
>
> I tried it and got the following error when I did C-c C-m parbox RET RET:
> Debugger entered--Lisp error: (void-variable exit-mark)
>   TeX-argument-insert("" nil)
>   TeX-arg-string(nil "Width")
>   TeX-parse-argument(nil "Width")
>   TeX-parse-arguments(([TeX-arg-tb] ["Height"] [TeX-arg-tb "Inner position"]
> "Width" t))
>   TeX-parse-macro("parbox" ([TeX-arg-tb] ["Height"] [TeX-arg-tb "Inner
> position"] "Width" t))
>   TeX-insert-macro("parbox")
>   funcall-interactively(TeX-insert-macro "parbox")
>   call-interactively(TeX-insert-macro nil nil)
>   command-execute(TeX-insert-macro)
>
> We still have to deal with the variable `exit-mark'.

I just did so (-> TeX-exit-mark), and also with last-optional-rejected
(-> TeX-last-optional-rejected).  Inserting a parbox works again. :-)

I'm sure there is other breakage hidden to be revealed.

Thanks for giving it a try!

  Tassilo

Re: Onwards to lexical binding (attempt 1)

[Ikumi Keita]

Hi Tassilo,

> Tassilo Horn  writes:
> I've done that now, too.  I still can run LaTeX and call the Evince
> viewer (TeX-evince-sync-view and TeX-pdf-tools-sync-view don't rely on
> `file' anymore but use `TeX-active-master' as suggested by Al Haji-Ali),
> so it seems I haven't broken everything [but possibly something I didn't
> test]...

I tried it and got the following error when I did C-c C-m parbox RET RET:
Debugger entered--Lisp error: (void-variable exit-mark)
  TeX-argument-insert("" nil)
  TeX-arg-string(nil "Width")
  TeX-parse-argument(nil "Width")
  TeX-parse-arguments(([TeX-arg-tb] ["Height"] [TeX-arg-tb "Inner position"] 
"Width" t))
  TeX-parse-macro("parbox" ([TeX-arg-tb] ["Height"] [TeX-arg-tb "Inner 
position"] "Width" t))
  TeX-insert-macro("parbox")
  funcall-interactively(TeX-insert-macro "parbox")
  call-interactively(TeX-insert-macro nil nil)
  command-execute(TeX-insert-macro)

We still have to deal with the variable `exit-mark'.

Regards,
Ikumi Keita

Re: Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Tassilo Horn  writes:

> We still have such issues in tex.el with the variable `file' and some
> others.

I've done that now, too.  I still can run LaTeX and call the Evince
viewer (TeX-evince-sync-view and TeX-pdf-tools-sync-view don't rely on
`file' anymore but use `TeX-active-master' as suggested by Al Haji-Ali),
so it seems I haven't broken everything [but possibly something I didn't
test]...

Bye,
Tassilo

Onwards to lexical binding (attempt 1)

[Tassilo Horn]

Hi all,

on the new branch lexical-binding-attempt-1, I've squashed all
"reference to free variable" warnings in latex.el and context.el by
declaring them properly with defvars and with a prefix, and changing all
users.

This affects in latex.el

  LaTeX-done-mark
  LaTeX-level
  LaTeX-name
  LaTeX-title
  LaTeX-toc

and in context.el

  ConTeXt-done-mark
  ConTeXt-level
  ConTeXt-name
  ConTeXt-title
  ConTeXt-reference

which were previously neither prefixed nor declared.

Of course, that is an incompatible change which will break user
LaTeX-section-hook functions which accessed those unprefixed variables.
But if we want to go lexical-binding (which I think we should), there's
no way around that.

Of course, I did not test too much (and ConTeXt not at all), so it would
be great if you could give it a whirl.  There are chances that I might
have changed an occurrence which I shouldn't have.

We still have such issues in tex.el with the variable `file' and some
others.  If someone wants to take care of that in a similar vein, feel
free to give it a shot.

Bye,
Tassilo