Re: [PATCH] Always use \newcommandx instead of \def

2023-10-19 Thread Enrico Forestieri

On Wed, Oct 18, 2023 at 02:05:51AM +, Isaac Oscar Gariano wrote:


So sorry about that, I should really have looked at the issue tracker. 
But yes, it will probably break lots of pre-existing documents. So I've 
uploaded a new version of my patch that is backwards compatible.
I've added a checkbox at the bottom of Document Settings -> Math 
Options to always use \newcommandx. The checkbox defaults to off, so 
it's an opt in feature. Alternatively, I could modify the lyx2lyx 
system so that files from an older format version have it off by 
default, but new versions have it on by default (which is probably what 
it should be).


I've attached three example lyx files that try and redefine the 
\section command; the only difference between def.lyx and new.lyx is 
whether the checkbox is checked or not; renew.lyx shows how to 
suecesfully do the redefinition.


Please, attach patch and examples to the bug tracker 
(https://www.lyx.org/trac/ticket/11185) so that they won't be forgotten.


--
Enrico
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-18 Thread Richard Kimberly Heck

On 10/17/23 14:50, Enrico Forestieri wrote:

On Tue, Oct 17, 2023 at 08:29:12PM +0200, Enrico Forestieri wrote:


On Tue, Oct 17, 2023 at 01:10:23PM -0400, Richard Kimberly Heck wrote:


On 10/16/23 21:22, Isaac Oscar Gariano wrote:
Currently \def is used for all math macros that have no option 
arguments. This has caused me hard to debug errors because it 
silently clobbers pre-existing LaTeX commands. On the other hand, 
\newcommandx is used if there are optional arguments, and this 
gives a helpful error message if the command is already defined. 
(if you really do want to override the command, you can just put 
\let\mycommand=\undefined in your LaTeX preamble, although with 
unicode-math I often have to put this in an \AtBeginDocument).


This very simple patch just makes LyX always output \newcommandx 
instead of \def for math macros.
Unfortunately, this means if you want to have any math macros 
you'll always need the xargs package (but it's a small package 
release all they way back in 2008 and is included in TeXlive), 
hopefully this won't annoy users too much.


Since we already depend upon xargs, this won't be a problem.


However, I think this might count as a format change, because it 
changes the exported LaTeX. If so, then it would have to wait for 
2.5.0. But I'm not sure about this, and perhaps the people who 
actually know about this will think it's safe. I'd like to hear.


Independently from that, this is a change that needs a lot of testing 
and it is risky to perform near a release.


Yes, I decided not to mention that, since the format change decides it 
anyway.


It's unfortunate, but perhaps we can get 2.5.0 out sooner...

Riki


--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-18 Thread Scott Kostyshak
On Wed, Oct 18, 2023 at 03:35:33PM +, Isaac Oscar Gariano wrote:
> The new.lyx and renew.lyx files won't load if you haven't applied my patch to 
> the master branch  and recompiled lyx.
> 
> The def.lyx file should however not require my patch (at least on the version 
> of master I had checked out)

Ah, I understand now. Thanks!

> And if you're happy to handle the tests, that would be great.

OK, I'll be happy to handle them once the patch is figured out. I still don't 
understand the technical details of it.

> I should probably learn how to do it eventually though if I keep making 
> changes to LyX, but I am actually supposed to be doing work at the moment.

:)


signature.asc
Description: PGP signature
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-18 Thread Isaac Oscar Gariano
The new.lyx and renew.lyx files won't load if you haven't applied my patch to 
the master branch  and recompiled lyx.

The def.lyx file should however not require my patch (at least on the version 
of master I had checked out)

And if you're happy to handle the tests, that would be great. I should probably 
learn how to do it eventually though if I keep making changes to LyX, but I am 
actually supposed to be doing work at the moment.

— Isaac Oscar Gariano
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-18 Thread Scott Kostyshak
On Wed, Oct 18, 2023 at 02:05:51AM +, Isaac Oscar Gariano wrote:

> I've attached three example lyx files that try and redefine the \section​ 
> command; the only difference between def.lyx​ and new.lyx​ is whether the 
> checkbox is checked or not; renew.lyx​ shows how to suecesfully do the 
> redefinition.

Thanks for these example files! For some reason I get a "header error" when I 
try to open them in LyX.

> I'm happy to integrate this with whatever automated testing setup LyX has, if 
> someone could point me to any documentation for it, that would be greatly 
> appreciated!.

I can take care of this. The testing system is a bit complicated. If you are 
very curious, I can direct you to the documentation, but otherwise I will save 
you the trouble.

Scott


signature.asc
Description: PGP signature
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-17 Thread Isaac Oscar Gariano
So sorry about that, I should really have looked at the issue tracker. But yes, 
it will probably break lots of pre-existing documents. So I've uploaded a new 
version of my patch that is backwards compatible.
I've added a checkbox at the bottom of Document Settings -> Math Options to 
always use \newcommandx​. The checkbox defaults to off, so it's an opt in 
feature. Alternatively, I could modify the lyx2lyx system so that files from an 
older format version have it off by default, but new versions have it on by 
default (which is probably what it should be).

I've attached three example lyx files that try and redefine the \section​ 
command; the only difference between def.lyx​ and new.lyx​ is whether the 
checkbox is checked or not; renew.lyx​ shows how to suecesfully do the 
redefinition.
I'm happy to integrate this with whatever automated testing setup LyX has, if 
someone could point me to any documentation for it, that would be greatly 
appreciated!.

The def.lyx​ file will work on LyX's without my changes, the new.lyx​ and 
renew.lyx​ files require the new changes.
The main part of the LaTeX for def.lyx​ is as follows:

\usepackage{babel}
\begin{document}
\newcommandx\section[0][usedefault, 
addprefix=\global]{\ensuremath{\mathsection}}%

Please See $\mathsection1$.

\section{Hello World}
\end{document}

The new.lyx​ file is the same, but changes the \def​​ line above to:
\newcommandx\section[0][usedefault, addprefix=\global]{\mathsection}%



Both versions give LaTeX errors, the first one:
! Missing $ inserted.

$
l.12 \section
 {Hello World}



And the second one:

! LaTeX Error: Command \section already defined.
   Or name \end... illegal, see p.192 of the manual.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H   for immediate help.
 ...

l.9 ...sedefault, addprefix=\global]{\mathsection}
  %


Clearly the second error message is much more helpfull.
Of course if you know what you're doing and really do want to redefine 
\section​ you can do it by first undefineing it in your preamble, which is what 
renew.lyx​ does (as well as make the new \section​ work outside math mode):

\makeatletter
%% User specified LaTeX commands.
\let\section=\undefined

\makeatother

\usepackage{babel}
\begin{document}
\newcommandx\section[0][usedefault, 
addprefix=\global]{\ensuremath{\mathsection}}%

Please See $\section1$.

\section{Hello World}
\end{document}

This version compiles fine.
Interstingly, sometimes you'll have to wrap the let  in \AtBeginDocument​ for 
example if you're redefining math commands defined by unicode-math.
— Isaac Oscar Gariano​


From: lyx-devel  on behalf of Enrico 
Forestieri 
Sent: Wednesday, 18 October 2023 7:50 AM
To: lyx-devel@lists.lyx.org 
Subject: Re: [PATCH] Always use \newcommandx instead of \def

On Tue, Oct 17, 2023 at 08:29:12PM +0200, Enrico Forestieri wrote:
>
>On Tue, Oct 17, 2023 at 01:10:23PM -0400, Richard Kimberly Heck wrote:
>>
>>On 10/16/23 21:22, Isaac Oscar Gariano wrote:
>>>Currently \def is used for all math macros that have no option
>>>arguments. This has caused me hard to debug errors because it
>>>silently clobbers pre-existing LaTeX commands. On the other hand,
>>>\newcommandx is used if there are optional arguments, and this
>>>gives a helpful error message if the command is already defined.
>>>(if you really do want to override the command, you can just put
>>>\let\mycommand=\undefined in your LaTeX preamble, although with
>>>unicode-math I often have to put this in an \AtBeginDocument).
>>>
>>>This very simple patch just makes LyX always output \newcommandx
>>>instead of \def for math macros.
>>>Unfortunately, this means if you want to have any math macros
>>>you'll always need the xargs package (but it's a small package
>>>release all they way back in 2008 and is included in TeXlive),
>>>hopefully this won't annoy users too much.
>>
>>Since we already depend upon xargs, this won't be a problem.
>>
>>
>>However, I think this might count as a format change, because it
>>changes the exported LaTeX. If so, then it would have to wait for
>>2.5.0. But I'm not sure about this, and perhaps the people who
>>actually know about this will think it's safe. I'd like to hear.
>
>Independently from that, this is a change that needs a lot of testing
>and it is risky to perform near a release.
>
>>You might want to have a look at 76dc2c0d3002db, which is where this
>>code was introduced. It looks to me as if the previous code was
>>actually very similar to what you have, but minus the prefix=\global
>>part, and that we could then just have added that. It's a bit
>>

Re: [PATCH] Always use \newcommandx instead of \def

2023-10-17 Thread Enrico Forestieri

On Tue, Oct 17, 2023 at 08:29:12PM +0200, Enrico Forestieri wrote:


On Tue, Oct 17, 2023 at 01:10:23PM -0400, Richard Kimberly Heck wrote:


On 10/16/23 21:22, Isaac Oscar Gariano wrote:
Currently \def is used for all math macros that have no option 
arguments. This has caused me hard to debug errors because it 
silently clobbers pre-existing LaTeX commands. On the other hand, 
\newcommandx is used if there are optional arguments, and this 
gives a helpful error message if the command is already defined. 
(if you really do want to override the command, you can just put 
\let\mycommand=\undefined in your LaTeX preamble, although with 
unicode-math I often have to put this in an \AtBeginDocument).


This very simple patch just makes LyX always output \newcommandx 
instead of \def for math macros.
Unfortunately, this means if you want to have any math macros 
you'll always need the xargs package (but it's a small package 
release all they way back in 2008 and is included in TeXlive), 
hopefully this won't annoy users too much.


Since we already depend upon xargs, this won't be a problem.


However, I think this might count as a format change, because it 
changes the exported LaTeX. If so, then it would have to wait for 
2.5.0. But I'm not sure about this, and perhaps the people who 
actually know about this will think it's safe. I'd like to hear.


Independently from that, this is a change that needs a lot of testing 
and it is risky to perform near a release.


You might want to have a look at 76dc2c0d3002db, which is where this 
code was introduced. It looks to me as if the previous code was 
actually very similar to what you have, but minus the prefix=\global 
part, and that we could then just have added that. It's a bit 
puzzling to me why Stefan did it the way he did, but he's long since 
not around to answer that question.


I seem to remember that he was actually forced to abandon the 
\newcommandx approach due to some technical reason. Maybe there is a 
bug report with details about this.


Anyway, this actually is the following enhancement request:
https://www.lyx.org/trac/ticket/11185

--
Enrico
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-17 Thread Enrico Forestieri

On Tue, Oct 17, 2023 at 01:10:23PM -0400, Richard Kimberly Heck wrote:


On 10/16/23 21:22, Isaac Oscar Gariano wrote:
Currently \def is used for all math macros that have no option 
arguments. This has caused me hard to debug errors because it 
silently clobbers pre-existing LaTeX commands. On the other hand, 
\newcommandx is used if there are optional arguments, and this gives 
a helpful error message if the command is already defined. (if you 
really do want to override the command, you can just put 
\let\mycommand=\undefined in your LaTeX preamble, although with 
unicode-math I often have to put this in an \AtBeginDocument).


This very simple patch just makes LyX always output \newcommandx 
instead of \def for math macros.
Unfortunately, this means if you want to have any math macros you'll 
always need the xargs package (but it's a small package release all 
they way back in 2008 and is included in TeXlive), hopefully this 
won't annoy users too much.


Since we already depend upon xargs, this won't be a problem.


However, I think this might count as a format change, because it 
changes the exported LaTeX. If so, then it would have to wait for 
2.5.0. But I'm not sure about this, and perhaps the people who 
actually know about this will think it's safe. I'd like to hear.


Independently from that, this is a change that needs a lot of testing 
and it is risky to perform near a release.


You might want to have a look at 76dc2c0d3002db, which is where this 
code was introduced. It looks to me as if the previous code was 
actually very similar to what you have, but minus the prefix=\global 
part, and that we could then just have added that. It's a bit puzzling 
to me why Stefan did it the way he did, but he's long since not around 
to answer that question.


I seem to remember that he was actually forced to abandon the 
\newcommandx approach due to some technical reason. Maybe there is a bug 
report with details about this.


--
Enrico
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-17 Thread Richard Kimberly Heck

On 10/16/23 21:22, Isaac Oscar Gariano wrote:
Currently \def is used for all math macros that have no option 
arguments. This has caused me hard to debug errors because it silently 
clobbers pre-existing LaTeX commands. On the other hand, \newcommandx 
is used if there are optional arguments, and this gives a helpful 
error message if the command is already defined. (if you really do 
want to override the command, you can just put 
\let\mycommand=\undefined in your LaTeX preamble, although with 
unicode-math I often have to put this in an \AtBeginDocument).


This very simple patch just makes LyX always output \newcommandx 
instead of \def for math macros.
Unfortunately, this means if you want to have any math macros you'll 
always need the xargs package (but it's a small package release all 
they way back in 2008 and is included in TeXlive), hopefully this 
won't annoy users too much.


Since we already depend upon xargs, this won't be a problem.


However, I think this might count as a format change, because it changes 
the exported LaTeX. If so, then it would have to wait for 2.5.0. But I'm 
not sure about this, and perhaps the people who actually know about this 
will think it's safe. I'd like to hear.



You might want to have a look at 76dc2c0d3002db, which is where this 
code was introduced. It looks to me as if the previous code was actually 
very similar to what you have, but minus the prefix=\global part, and 
that we could then just have added that. It's a bit puzzling to me why 
Stefan did it the way he did, but he's long since not around to answer 
that question.



Riki

-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Re: [PATCH] Always use \newcommandx instead of \def

2023-10-17 Thread Scott Kostyshak
On Tue, Oct 17, 2023 at 01:22:26AM +, Isaac Oscar Gariano wrote:
> Currently \def is used for all math macros that have no option arguments. 
> This has caused me hard to debug errors because it silently clobbers 
> pre-existing LaTeX commands. On the other hand, \newcommandx is used if there 
> are optional arguments, and this gives a helpful error message if the command 
> is already defined. (if you really do want to override the command, you can 
> just put \let\mycommand=\undefined in your LaTeX preamble, although with 
> unicode-math I often have to put this in an \AtBeginDocument).
> 
> This very simple patch just makes LyX always output \newcommandx instead of 
> \def for math macros.
> Unfortunately, this means if you want to have any math macros you'll always 
> need the xargs package (but it's a small package release all they way back in 
> 2008 and is included in TeXlive), hopefully this won't annoy users too much.
> 
> If you don't like this due to the dependency on the xargs package:
> I'm happy to modify it, to for example:
> 
>   *   Make this patch togglable with a checkbox in the GUI (default will of 
> course by the previous behaviour)
>   *   Execute a dummy \newcommand to get the error message, then do the \def 
> as before
> 
> — Isaac Oscar Gariano​

Thank you Isaac! I'm not too familiar with these technical details so I
can't comment on the patch. It might help if you could send a few
minimal .lyx documents that illustrate the problem with the current
mechanism. In addition to making the current problem very clear, those
.lyx files would also be helpful to add to our tests if we move towards
an implementation similar to what you have in mind. If we add the
feature and the test files then we can protect against regressions.

Thanks for your work on this!

Scott


signature.asc
Description: PGP signature
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel