Re: Patches for Python scripts
On Mon, 1 Feb 2021 at 15:31, José Abílio Matos wrote: > On Monday, February 1, 2021 2:42:43 AM WET Thibaut Cuvelier wrote: > > > More generally, what about the other patches? Are formatting changes > > > considered risky? What about Joel's suggestions? > > My issue is not about the formatting changes, I can live with them even if > in some cases I think that it is a matter of taste. > > The example about documentation strings is striking, why do I need triple > quotes if the documentation string will only span a single line? > Because it's really preferred for Python: https://www.python.org/dev/peps/pep-0257/#what-is-a-docstring > My main doubt, that I still did not get any feedback, comes from the > regular expression changes. Those are in no way formatting changes... > > And even if the current code is faulty, i.e. it works by accident, the > proposed changes are wrong. > There is no reason why ] should be escaped in a regex, as far as I know (and PyCharm doesn't know either). The change should not have any impact on the correctness of the code. I've just pushed the patches that were not objected to, plus the one for Java that was discussed at length here. -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Monday, February 1, 2021 2:42:43 AM WET Thibaut Cuvelier wrote: > More generally, what about the other patches? Are formatting changes > considered risky? What about Joel's suggestions? My issue is not about the formatting changes, I can live with them even if in some cases I think that it is a matter of taste. The example about documentation strings is striking, why do I need triple quotes if the documentation string will only span a single line? My main doubt, that I still did not get any feedback, comes from the regular expression changes. Those are in no way formatting changes... And even if the current code is faulty, i.e. it works by accident, the proposed changes are wrong. -- José Abílio-- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Mon, Feb 01, 2021 at 03:42:43AM +0100, Thibaut Cuvelier wrote: > Thank you for testing on your side :)! What's frustrating with your report > is that this bug can only appear on Python 2, and that it will very likely > be dropped for the next release (I remember seeing people wanting to keep > it for LyX 2.4). Nevertheless, I'm trying to do a few things about it. Even if support for Python 2 is being discontinued, it is not going to disappear overnight. So, supporting it for some more time is right, IMHO. > What I've been doing: change the output when looking for programs, with a > more explicit "not in path" instead of "no", and the same for the registry > check; convert the result of winreg.QueryValueEx to a string (in Python 2, > it looks like it's a byte array, hence the error). If that doesn't work, I > really don't know what to do, apart from not telling the user what Java > binary was found. > > I'm attaching a differential patch (unnamed8) for the latest changes, plus > a complete updated patch that should replace the original 0013 patch. Does > it work for you? Yes! This works also in a cygwin shell. Thanks! ... checking for a java interpreter... +checking for "java"... not in path +checking for java: found in Windows registry, C:\Program Files (x86)\Java\jre1. 8.0_281\bin\java.exe ... > More generally, what about the other patches? Are formatting changes > considered risky? What about Joel's suggestions? I am leaving these to our python experts. -- Enrico -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Sun, 31 Jan 2021 at 12:39, Enrico Forestieri wrote:
> On Sun, Jan 31, 2021 at 04:22:39AM +0100, Thibaut Cuvelier wrote:
> > On Sat, 30 Jan 2021 at 12:26, Enrico Forestieri wrote:
> > > On Sat, Jan 30, 2021 at 05:31:05AM +0100, Thibaut Cuvelier wrote:
> > > > On Sat, 30 Jan 2021 at 02:31, Enrico Forestieri
> wrote:
> > > > >
> > > > > So, it seems that you have to check also for the 32-bit version.
> > > > >
> > > > Indeed. Would the attached patch solve the problem? I first look for
> a
> > > > 64-bit version before a 32-bit, just to favour newer things, but
> there is
> > > > no other reason to do so.
> > >
> > > The patch does not apply neither to current master nor to current
> stable
> > > (most probably it depends on previous patches):
> > >
> > > $ patch --dry-run -p1 <
> > > 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch
> > > checking file lib/configure.py
> > > Hunk #1 FAILED at 330.
> > > Hunk #2 succeeded at 403 (offset -14 lines).
> > > Hunk #3 succeeded at 1923 (offset -94 lines).
> > > 1 out of 3 hunks FAILED
> > >
> > > and I had to apply it manually. Before applying it, I took away java
> from
> > > PATH and reconfigured:
> > >
> > > ...
> > > checking for a java interpreter...
> > > +checking for "java"... no
> > > ...
> > >
> > > Correctly, java was not found (it is found when it is in PATH).
> > > Then I applied the patch an reconfigured again:
> > >
> > > ...
> > > checking for a java interpreter...
> > > +checking for "java"... no
> > > Traceback (most recent call last):
> > > File "C:\cygwin\win\python\lib\logging\__init__.py", line 870, in
> emit
> > > stream.write(ufs % msg)
> > > LookupError: unknown encoding: cp65001
> > > Logged from file configure.py, line 346
> > > ...
> > >
> > > I have a native Windows version of Python 2.7 installed in
> > > "C:\cygwin\win\python".
> > >
> > > So, it seems there is a problem with Python 2
> > >
> >
> > I just tried with Python 2.7.18 (64 bits), I cannot reproduce your
> problem.
> > What is exactly on your line 346? I guess it is something like
> logger.info,
> > a line that runs without problem on my side (Python 3.8 and 2.7).
>
> Yes, that's exactly "logger.info('+checking for java: ' + java_bin)".
> It seems a problem related to the locale. I was reconfiguring from a
> cygwin shell, whose locale is UTF-8, apparently something alien to
> Windows. I now tried reconfiguring from a cmd.exe window and the result is:
>
> ...
> checking for a java interpreter...
> +checking for "java"... no
> +checking for java: C:\Program Files (x86)\Java\jre1.8.0_281\bin\java.exe
> ...
>
> So, it works. However, to avoid confusion, you should avoid logging the
> first negative result. Or, maybe, on Windows report something like this:
>
> ...
> checking for a java interpreter...
> +checking for "java" in PATH... no
> +checking for "java" in registry... C:\Program Files
> (x86)\Java\jre1.8.0_281\bin\java.exe
> ...
>
> And, yes, I know making things work for everyone is exhausting...
>
> Thank you for your efforts.
>
Thank you for testing on your side :)! What's frustrating with your report
is that this bug can only appear on Python 2, and that it will very likely
be dropped for the next release (I remember seeing people wanting to keep
it for LyX 2.4). Nevertheless, I'm trying to do a few things about it.
What I've been doing: change the output when looking for programs, with a
more explicit "not in path" instead of "no", and the same for the registry
check; convert the result of winreg.QueryValueEx to a string (in Python 2,
it looks like it's a byte array, hence the error). If that doesn't work, I
really don't know what to do, apart from not telling the user what Java
binary was found.
I'm attaching a differential patch (unnamed8) for the latest changes, plus
a complete updated patch that should replace the original 0013 patch. Does
it work for you?
More generally, what about the other patches? Are formatting changes
considered risky? What about Joel's suggestions?
unnamed8.patch
Description: Binary data
0013-Configure-look-for-Java-in-the-registry-on-Windows.patch
Description: Binary data
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Sun, Jan 31, 2021 at 04:22:39AM +0100, Thibaut Cuvelier wrote:
> On Sat, 30 Jan 2021 at 12:26, Enrico Forestieri wrote:
> > On Sat, Jan 30, 2021 at 05:31:05AM +0100, Thibaut Cuvelier wrote:
> > > On Sat, 30 Jan 2021 at 02:31, Enrico Forestieri wrote:
> > > >
> > > > So, it seems that you have to check also for the 32-bit version.
> > > >
> > > Indeed. Would the attached patch solve the problem? I first look for a
> > > 64-bit version before a 32-bit, just to favour newer things, but there is
> > > no other reason to do so.
> >
> > The patch does not apply neither to current master nor to current stable
> > (most probably it depends on previous patches):
> >
> > $ patch --dry-run -p1 <
> > 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch
> > checking file lib/configure.py
> > Hunk #1 FAILED at 330.
> > Hunk #2 succeeded at 403 (offset -14 lines).
> > Hunk #3 succeeded at 1923 (offset -94 lines).
> > 1 out of 3 hunks FAILED
> >
> > and I had to apply it manually. Before applying it, I took away java from
> > PATH and reconfigured:
> >
> > ...
> > checking for a java interpreter...
> > +checking for "java"... no
> > ...
> >
> > Correctly, java was not found (it is found when it is in PATH).
> > Then I applied the patch an reconfigured again:
> >
> > ...
> > checking for a java interpreter...
> > +checking for "java"... no
> > Traceback (most recent call last):
> > File "C:\cygwin\win\python\lib\logging\__init__.py", line 870, in emit
> > stream.write(ufs % msg)
> > LookupError: unknown encoding: cp65001
> > Logged from file configure.py, line 346
> > ...
> >
> > I have a native Windows version of Python 2.7 installed in
> > "C:\cygwin\win\python".
> >
> > So, it seems there is a problem with Python 2
> >
>
> I just tried with Python 2.7.18 (64 bits), I cannot reproduce your problem.
> What is exactly on your line 346? I guess it is something like logger.info,
> a line that runs without problem on my side (Python 3.8 and 2.7).
Yes, that's exactly "logger.info('+checking for java: ' + java_bin)".
It seems a problem related to the locale. I was reconfiguring from a
cygwin shell, whose locale is UTF-8, apparently something alien to
Windows. I now tried reconfiguring from a cmd.exe window and the result is:
...
checking for a java interpreter...
+checking for "java"... no
+checking for java: C:\Program Files (x86)\Java\jre1.8.0_281\bin\java.exe
...
So, it works. However, to avoid confusion, you should avoid logging the
first negative result. Or, maybe, on Windows report something like this:
...
checking for a java interpreter...
+checking for "java" in PATH... no
+checking for "java" in registry... C:\Program Files
(x86)\Java\jre1.8.0_281\bin\java.exe
...
And, yes, I know making things work for everyone is exhausting...
Thank you for your efforts.
--
Enrico
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Sat, 30 Jan 2021 at 12:26, Enrico Forestieri wrote: > On Sat, Jan 30, 2021 at 05:31:05AM +0100, Thibaut Cuvelier wrote: > > On Sat, 30 Jan 2021 at 02:31, Enrico Forestieri wrote: > > > > > > So, it seems that you have to check also for the 32-bit version. > > > > > Indeed. Would the attached patch solve the problem? I first look for a > > 64-bit version before a 32-bit, just to favour newer things, but there is > > no other reason to do so. > > The patch does not apply neither to current master nor to current stable > (most probably it depends on previous patches): > > $ patch --dry-run -p1 < > 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch > checking file lib/configure.py > Hunk #1 FAILED at 330. > Hunk #2 succeeded at 403 (offset -14 lines). > Hunk #3 succeeded at 1923 (offset -94 lines). > 1 out of 3 hunks FAILED > > and I had to apply it manually. Before applying it, I took away java from > PATH and reconfigured: > > ... > checking for a java interpreter... > +checking for "java"... no > ... > > Correctly, java was not found (it is found when it is in PATH). > Then I applied the patch an reconfigured again: > > ... > checking for a java interpreter... > +checking for "java"... no > Traceback (most recent call last): > File "C:\cygwin\win\python\lib\logging\__init__.py", line 870, in emit > stream.write(ufs % msg) > LookupError: unknown encoding: cp65001 > Logged from file configure.py, line 346 > ... > > I have a native Windows version of Python 2.7 installed in > "C:\cygwin\win\python". > > So, it seems there is a problem with Python 2 > I just tried with Python 2.7.18 (64 bits), I cannot reproduce your problem. What is exactly on your line 346? I guess it is something like logger.info, a line that runs without problem on my side (Python 3.8 and 2.7). -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Sat, Jan 30, 2021 at 05:31:05AM +0100, Thibaut Cuvelier wrote: > On Sat, 30 Jan 2021 at 02:31, Enrico Forestieri wrote: > > > > So, it seems that you have to check also for the 32-bit version. > > > Indeed. Would the attached patch solve the problem? I first look for a > 64-bit version before a 32-bit, just to favour newer things, but there is > no other reason to do so. The patch does not apply neither to current master nor to current stable (most probably it depends on previous patches): $ patch --dry-run -p1 < 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch checking file lib/configure.py Hunk #1 FAILED at 330. Hunk #2 succeeded at 403 (offset -14 lines). Hunk #3 succeeded at 1923 (offset -94 lines). 1 out of 3 hunks FAILED and I had to apply it manually. Before applying it, I took away java from PATH and reconfigured: ... checking for a java interpreter... +checking for "java"... no ... Correctly, java was not found (it is found when it is in PATH). Then I applied the patch an reconfigured again: ... checking for a java interpreter... +checking for "java"... no Traceback (most recent call last): File "C:\cygwin\win\python\lib\logging\__init__.py", line 870, in emit stream.write(ufs % msg) LookupError: unknown encoding: cp65001 Logged from file configure.py, line 346 ... I have a native Windows version of Python 2.7 installed in "C:\cygwin\win\python". So, it seems there is a problem with Python 2. -- Enrico -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Sat, 30 Jan 2021 at 02:31, Enrico Forestieri wrote: > On Fri, Jan 29, 2021 at 11:40:29PM +0100, Thibaut Cuvelier wrote: > > Hi Thibaut, > > > - This way to find Java is quite common on Windows platforms (actually, > > it's a lot like a port of JavaCall.jl's relevant portion of code: > > https://github.com/JuliaInterop/JavaCall.jl). It looks like Oracle's JVM > > does the same at installation: > https://stackoverflow.com/a/9094539/1066843. > > Other JVMs mimic the same behaviour: > > https://github.com/AdoptOpenJDK/openjdk-installer/issues/64. > > Moreover, I don't think we can let beginner users on their own to > configure > > the PATH, we really should go an extra step to find a Java binary (while > > letting the PATH have preference over anything else, so that users can > > still choose which Java they want). > > Using things like \HKEY_CLASSES_ROOT\Installer\Products\ is, in my > opinion, > > the wrong way to check for Java in the registry: these identifiers seem > to > > vary a lot between versions of Java ( > > > https://docs.oracle.com/en/java/javase/15/install/installation-jdk-microsoft-windows-platforms.html#GUID-886D35CE-76AF-43B4-8C2F-7B34DB8B1666 > ). > > On my machine, I have no 4EA42A62D9304AC4784BF2238120180F, for instance. > > That was actually my point. I found that key by searching for java with > regedit after not finding JavaSoft under HKEY_LOCAL_MACHINE/SOFTWARE/ > > > I am attaching a new version of the patch based on > > > https://docs.oracle.com/javase/9/install/installation-jdk-and-jre-microsoft-windows-platforms.htm#JSJIG-GUID-C11500A9-252C-46FE-BB17-FC5A9528EAEB > . > > Does it work for you? Otherwise, would you have something else under > > HKEY_LOCAL_MACHINE\SOFTWARE\JavaSoft? > > I do not have HKEY_LOCAL_MACHINE\SOFTWARE\JavaSoft : > > $ ls -F /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/Java* > ls: No such file or directory > > but, after reading the stackoverflow link above, I found that I have: > > $ ls -F /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/WOW6432Node/JavaSoft/ > 'Auto Update'/ 'Java Update'/ sponsorsoffered > 'Java Plug-in'/ 'Java Web Start'/ > 'Java Runtime Environment'/ 'Java Web Start Caps'/ > > and: > > $ ls -F > /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/WOW6432Node/JavaSoft/Java\ > Runtime\ Environment/ > 1.8/ 1.8.0_281/ BrowserJavaVersion CurrentVersion > > So, it seems that you have to check also for the 32-bit version. > Indeed. Would the attached patch solve the problem? I first look for a 64-bit version before a 32-bit, just to favour newer things, but there is no other reason to do so. 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch Description: Binary data -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Fri, Jan 29, 2021 at 3:40 PM Thibaut Cuvelier wrote: > - The cosmetic changes are based on PEP8, which is the only style guide > for Python. Highly similar patches already went into the code base less > than a year ago ( > http://lists.lyx.org/pipermail/lyx-devel/2020-May/001464.html). Other > style guides for Python are just more precise than PEP8 (e.g., Google: > https://stackoverflow.com/a/2815311/1066843). I have yet to see an > important open-source project that does not use PEP8 style guide. It's as > unopiniated as opinion could get on Python. > For now, the Python scripts follow the WTF style guide: every committer > does what they prefer, without any kind of consistency. Python is very > different from other languages in that there is a real consensus in the > community on one style guide (PHP is going in the same direction, but it's > not yet as commonly used: https://www.php-fig.org/psr/psr-2/). I am not > proposing one style guide for C++ or Perl code, for instance. > Some food for thought based on recent experience... The idea of code style (and style application automation) is something that has been on my mind a lot lately. Applying the black utility [ https://github.com/psf/black] for Python and clang-format utility [ https://clang.llvm.org/docs/ClangFormat.html] for C/C++ to several projects I work on have helped reduce style discussion, ensure uniformity, and provide cognitive ease as code is considered. It's to the point that several projects have early CI static analysis that ensures conformance to the applicable style. While one may not love any particular style, having a standard does help with pattern recognition while reading code. Regards, Joel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Fri, Jan 29, 2021 at 11:40:29PM +0100, Thibaut Cuvelier wrote: Hi Thibaut, > - This way to find Java is quite common on Windows platforms (actually, > it's a lot like a port of JavaCall.jl's relevant portion of code: > https://github.com/JuliaInterop/JavaCall.jl). It looks like Oracle's JVM > does the same at installation: https://stackoverflow.com/a/9094539/1066843. > Other JVMs mimic the same behaviour: > https://github.com/AdoptOpenJDK/openjdk-installer/issues/64. > Moreover, I don't think we can let beginner users on their own to configure > the PATH, we really should go an extra step to find a Java binary (while > letting the PATH have preference over anything else, so that users can > still choose which Java they want). > Using things like \HKEY_CLASSES_ROOT\Installer\Products\ is, in my opinion, > the wrong way to check for Java in the registry: these identifiers seem to > vary a lot between versions of Java ( > https://docs.oracle.com/en/java/javase/15/install/installation-jdk-microsoft-windows-platforms.html#GUID-886D35CE-76AF-43B4-8C2F-7B34DB8B1666). > On my machine, I have no 4EA42A62D9304AC4784BF2238120180F, for instance. That was actually my point. I found that key by searching for java with regedit after not finding JavaSoft under HKEY_LOCAL_MACHINE/SOFTWARE/ > I am attaching a new version of the patch based on > https://docs.oracle.com/javase/9/install/installation-jdk-and-jre-microsoft-windows-platforms.htm#JSJIG-GUID-C11500A9-252C-46FE-BB17-FC5A9528EAEB. > Does it work for you? Otherwise, would you have something else under > HKEY_LOCAL_MACHINE\SOFTWARE\JavaSoft? I do not have HKEY_LOCAL_MACHINE\SOFTWARE\JavaSoft : $ ls -F /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/Java* ls: No such file or directory but, after reading the stackoverflow link above, I found that I have: $ ls -F /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/WOW6432Node/JavaSoft/ 'Auto Update'/ 'Java Update'/ sponsorsoffered 'Java Plug-in'/ 'Java Web Start'/ 'Java Runtime Environment'/ 'Java Web Start Caps'/ and: $ ls -F /proc/registry/HKEY_LOCAL_MACHINE/SOFTWARE/WOW6432Node/JavaSoft/Java\ Runtime\ Environment/ 1.8/ 1.8.0_281/ BrowserJavaVersion CurrentVersion So, it seems that you have to check also for the 32-bit version. -- Enrico -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On 1/29/21 7:39 AM, Pavel Sanda wrote: > On Fri, Jan 29, 2021 at 11:07:43AM +, José Abílio Matos wrote: >> With the extent of the patches I fear that there could be bugs (unintended >> changes) lurking specially in relation to python 2. > Riki's call, but I don't think this is the best time for python scripts > overhaul. > Except the changes fixing real bugs we should IMHO postpone these to post 2.4 > world. I would agree with that. We don't want to do anything risky at this stage. Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
I will be answering most concerns here, in no particular order. I think it will simplify the next messages. - This is far from an overhaul of the script. The logic did not really change, neither the code structure. I have ideas on how to do it, and I could do it if you are OK with it, but I completely agree that larger changes should be delayed to the next version (or 2.4 should be delayed by a few months). This should also be the occasion to get rid of Python 2 completely, that would make the code much clearer. - The cosmetic changes are based on PEP8, which is the only style guide for Python. Highly similar patches already went into the code base less than a year ago (http://lists.lyx.org/pipermail/lyx-devel/2020-May/001464.html). Other style guides for Python are just more precise than PEP8 (e.g., Google: https://stackoverflow.com/a/2815311/1066843). I have yet to see an important open-source project that does not use PEP8 style guide. It's as unopiniated as opinion could get on Python. For now, the Python scripts follow the WTF style guide: every committer does what they prefer, without any kind of consistency. Python is very different from other languages in that there is a real consensus in the community on one style guide (PHP is going in the same direction, but it's not yet as commonly used: https://www.php-fig.org/psr/psr-2/). I am not proposing one style guide for C++ or Perl code, for instance. - This way to find Java is quite common on Windows platforms (actually, it's a lot like a port of JavaCall.jl's relevant portion of code: https://github.com/JuliaInterop/JavaCall.jl). It looks like Oracle's JVM does the same at installation: https://stackoverflow.com/a/9094539/1066843. Other JVMs mimic the same behaviour: https://github.com/AdoptOpenJDK/openjdk-installer/issues/64. Moreover, I don't think we can let beginner users on their own to configure the PATH, we really should go an extra step to find a Java binary (while letting the PATH have preference over anything else, so that users can still choose which Java they want). Using things like \HKEY_CLASSES_ROOT\Installer\Products\ is, in my opinion, the wrong way to check for Java in the registry: these identifiers seem to vary a lot between versions of Java ( https://docs.oracle.com/en/java/javase/15/install/installation-jdk-microsoft-windows-platforms.html#GUID-886D35CE-76AF-43B4-8C2F-7B34DB8B1666). On my machine, I have no 4EA42A62D9304AC4784BF2238120180F, for instance. I am attaching a new version of the patch based on https://docs.oracle.com/javase/9/install/installation-jdk-and-jre-microsoft-windows-platforms.htm#JSJIG-GUID-C11500A9-252C-46FE-BB17-FC5A9528EAEB. Does it work for you? Otherwise, would you have something else under HKEY_LOCAL_MACHINE\SOFTWARE\JavaSoft? On Fri, 29 Jan 2021 at 14:41, Kornel Benko wrote: > Am Fri, 29 Jan 2021 13:39:19 +0100 > schrieb Pavel Sanda : > > > On Fri, Jan 29, 2021 at 11:07:43AM +, José Abílio Matos wrote: > > > With the extent of the patches I fear that there could be bugs > (unintended > > > changes) lurking specially in relation to python 2. > > > > Riki's call, but I don't think this is the best time for python scripts > overhaul. > > Except the changes fixing real bugs we should IMHO postpone these to > post 2.4 world. > > > > Pavel > > +1 > > Kornel > -- > lyx-devel mailing list > lyx-devel@lists.lyx.org > http://lists.lyx.org/mailman/listinfo/lyx-devel > 0013-Configure-look-for-Java-in-the-registry-on-Windows.patch Description: Binary data -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
Am Fri, 29 Jan 2021 13:39:19 +0100 schrieb Pavel Sanda : > On Fri, Jan 29, 2021 at 11:07:43AM +, José Abílio Matos wrote: > > With the extent of the patches I fear that there could be bugs (unintended > > changes) lurking specially in relation to python 2. > > Riki's call, but I don't think this is the best time for python scripts > overhaul. > Except the changes fixing real bugs we should IMHO postpone these to post 2.4 > world. > > Pavel +1 Kornel pgpHG7SItevQQ.pgp Description: Digitale Signatur von OpenPGP -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Fri, Jan 29, 2021 at 11:07:43AM +, José Abílio Matos wrote: > With the extent of the patches I fear that there could be bugs (unintended > changes) lurking specially in relation to python 2. Riki's call, but I don't think this is the best time for python scripts overhaul. Except the changes fixing real bugs we should IMHO postpone these to post 2.4 world. Pavel -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Friday, January 29, 2021 11:48:40 AM WET Kornel Benko wrote:
> Right, the last one should be escaped.
> Given the line
> # \DeclareLaTeXClass[revtex,revtex.sty]{REVTeX (Obsolete Version)}
> and the original regex
>
> '\\s*#\\s*DeclareLaTeXClass\\s*(\[([^,]*)(,.*)*\])*\\s*{(.*)}\\s*$' the
> result in found(1) would be "revtex"
> and found(2) would be the rest inside the escaped brackets, e.g.
> "revtex.sty"
>
> Not escaping the last ']' probably not intended.
> (I would escape also '{' and '}' if that were a perl regex)
Now that you mention it the code should use raw strings in order to avoid
doubling the backslashes, e.g. in order to match \DeclareLaTeXclass the code
has "DeclareLaTeXclass" because the backslash needs to be escaped twice
the first time for the regular expression and then each needs to be escaped
again for python.
Using raw strings the results would be r"\\DeclareLaTeXclass" more in line
with what we usually write. :-)
This is a detour that come from the code analysis and it does not come from
Thibault's changes.
--
José Abílio--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
Am Fri, 29 Jan 2021 11:07:43 +
schrieb José Abílio Matos :
> On Friday, January 29, 2021 5:05:19 AM WET Richard Kimberly Heck wrote:
> > José, can you look at these please?
> >
> > Riki
>
> I think that Thibaut wants to impose black[1] on us. :-D
>
> Most of the changes are cosmetic, e.g. one import per line, or to use the
> same
> indentation for documentation.
>
> The only bits where I am unsure are on the regular expressions (I am trimming
> the expression to make it easier to read):
> -declare = re.compile('\\s*(\[([^,]*)(,.*)*\])*$')
> +declare = re.compile('\\s*(\[([^,]*)(,.*)*])*$')
Right, the last one should be escaped.
Given the line
# \DeclareLaTeXClass[revtex,revtex.sty]{REVTeX (Obsolete Version)}
and the original regex
'\\s*#\\s*DeclareLaTeXClass\\s*(\[([^,]*)(,.*)*\])*\\s*{(.*)}\\s*$'
the result in found(1) would be "revtex"
and found(2) would be the rest inside the escaped brackets, e.g. "revtex.sty"
Not escaping the last ']' probably not intended.
(I would escape also '{' and '}' if that were a perl regex)
> The left bracket is escaped but not the right one. What do other, more
> knowledgeable about the black magic of regular expressions think?
>
> The are other issues like mistakes in the documentation strings. For the
> moment those are harmless, I am referring to e.g. "\c" since \c it is not an
> escape sequence it gets transformed to "\\c".
>
>
> With the extent of the patches I fear that there could be bugs (unintended
> changes) lurking specially in relation to python 2.
>
> In particular this just reinforces my feeling that configure.py should get an
> overhaul. :-)
>
> This is my preliminary overview. I will keep looking into the patches.
>
>
> [1] From Henry Ford's T model "You can choose any color as long as it is
> black".
> https://black.readthedocs.io/en/stable/
>
pgppUSmSKjpVK.pgp
Description: Digitale Signatur von OpenPGP
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Friday, January 29, 2021 3:51:12 AM WET Thibaut Cuvelier wrote: > Dear list, > > While working on the ePub output, I ran through many Python scripts. I > corrected a few bgs and a lot of formatting, plus one feature (find Java in > the Windows registry in configure.py), here are the patches. May I push > them on the master branch? There are things that I like on the patches that fix real bugs, there are others that are cosmetic and in same case are white space changes. I am ignoring the java detection part already discussed by Enrico. Patches that should go in (they fix real bugs): * 10 (this is a matter of principle) * 7 * 5 * 6 * 12 (a good idea, that could be used elsewhere) I have doubts: * 11 (the regex changes) I am neutral on all the other. These changes are opinionated changes that change formatting mostly. BTW to continue with the joke regarding black there is a new package called blue: https://pypi.org/project/blue/ -- José Abílio-- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Friday, January 29, 2021 5:05:19 AM WET Richard Kimberly Heck wrote:
> José, can you look at these please?
>
> Riki
I think that Thibaut wants to impose black[1] on us. :-D
Most of the changes are cosmetic, e.g. one import per line, or to use the same
indentation for documentation.
The only bits where I am unsure are on the regular expressions (I am trimming
the expression to make it easier to read):
-declare = re.compile('\\s*(\[([^,]*)(,.*)*\])*$')
+declare = re.compile('\\s*(\[([^,]*)(,.*)*])*$')
The left bracket is escaped but not the right one. What do other, more
knowledgeable about the black magic of regular expressions think?
The are other issues like mistakes in the documentation strings. For the
moment those are harmless, I am referring to e.g. "\c" since \c it is not an
escape sequence it gets transformed to "\\c".
With the extent of the patches I fear that there could be bugs (unintended
changes) lurking specially in relation to python 2.
In particular this just reinforces my feeling that configure.py should get an
overhaul. :-)
This is my preliminary overview. I will keep looking into the patches.
[1] From Henry Ford's T model "You can choose any color as long as it is
black".
https://black.readthedocs.io/en/stable/
--
José Abílio--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On 1/29/21 4:41 AM, Enrico Forestieri wrote: > On Fri, Jan 29, 2021 at 04:51:12AM +0100, Thibaut Cuvelier wrote: >> While working on the ePub output, I ran through many Python scripts. I >> corrected a few bgs and a lot of formatting, plus one feature (find Java in >> the Windows registry in configure.py), here are the patches. May I push >> them on the master branch? > As regarda java, I have it installed but it is not found. I don't have the > registry keys that are searched for. A key containing a usable path is > \HKEY_CLASSES_ROOT\Installer\Products\4EA42A62D9304AC4784BF2238120180F\ProductIcon > and the fact that it *should* be something related to java is the fact that > \HKEY_CLASSES_ROOT\Installer\Products\4EA42A62D9304AC4784BF2238120180F\ProductName > contains "Java 8 Update 281". > > However, the java installer updates the PATH env var, so that I can > simply access the java executables from any command line. > > I discourage using ad hoc techniques that only work on some configurations. I would agree with that. But of course that makes it quite difficult to detect certain programs on Windows. I guess we have to live with that, then. I'll omit the usual complaints about Windows Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On Fri, Jan 29, 2021 at 04:51:12AM +0100, Thibaut Cuvelier wrote: > > While working on the ePub output, I ran through many Python scripts. I > corrected a few bgs and a lot of formatting, plus one feature (find Java in > the Windows registry in configure.py), here are the patches. May I push > them on the master branch? As regarda java, I have it installed but it is not found. I don't have the registry keys that are searched for. A key containing a usable path is \HKEY_CLASSES_ROOT\Installer\Products\4EA42A62D9304AC4784BF2238120180F\ProductIcon and the fact that it *should* be something related to java is the fact that \HKEY_CLASSES_ROOT\Installer\Products\4EA42A62D9304AC4784BF2238120180F\ProductName contains "Java 8 Update 281". However, the java installer updates the PATH env var, so that I can simply access the java executables from any command line. I discourage using ad hoc techniques that only work on some configurations. -- Enrico -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On 1/28/21 10:51 PM, Thibaut Cuvelier wrote: > Dear list, > > While working on the ePub output, I ran through many Python scripts. I > corrected a few bgs and a lot of formatting, plus one feature (find > Java in the Windows registry in configure.py), here are the patches. > May I push them on the master branch? José, can you look at these please? Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
Re: Patches for Python scripts
On 1/28/21 10:51 PM, Thibaut Cuvelier wrote: > Dear list, > > While working on the ePub output, I ran through many Python scripts. I > corrected a few bgs and a lot of formatting, plus one feature (find > Java in the Windows registry in configure.py), here are the patches. > May I push them on the master branch? José knows more about Python than any of us (and some of us know a fair bit!). Let's see what he has to say. Riki -- lyx-devel mailing list lyx-devel@lists.lyx.org http://lists.lyx.org/mailman/listinfo/lyx-devel
