Re: [LyX/master] lyx2lyx: Fix trim_eol_binary() to strip \r on CRLF files

On Saturday, August 8th, 2026 at 9:27 PM, José Matos <[email protected]> wrote:

> On Tue, 2026-08-04 at 16:43 +0000, Udi Fogiel wrote:
> > commit e4245896595c733e5e85984b41ea21e158c5a52c
> > Author: Udi Fogiel <[email protected]>
> > Date:   Tue Aug 4 19:23:54 2026 +0300
> >
> >     lyx2lyx: Fix trim_eol_binary() to strip \r on CRLF files
> >
> >     line[-2:-1] == 13 compares a bytes slice to an int,
> >     which is always False in Python 3
> > ---
> >  lib/lyx2lyx/LyX.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/lyx2lyx/LyX.py b/lib/lyx2lyx/LyX.py
> > index ab3e915859..ce27d27f95 100644
> > --- a/lib/lyx2lyx/LyX.py
> > +++ b/lib/lyx2lyx/LyX.py
> > @@ -191,7 +191,7 @@ def trim_eol_binary(line):
> >      if line[-1] != 10 and line[-1] != 13:
> >          # May happen for the last line of a document
> >          return line
> > -    if line[-2:-1] == 13:
> > +    if len(line) >= 2 and line[-2] == 13:
> >          return line[:-2]
> >      else:
> >          return line[:-1]
>
> I know that this is not your fault. It is mine. :-)

I would blame the fact that until this day
we do not have a consistent way to represent line ends.

> I suggest to make this code explicit, make the condition:
>
> ```
> if len(line) >= 2 and line[-2] == r'\r':
> ```
>
> so the idea is really to replace 13 by r'\r'.
>
> What do you think?
>
> BTW to keep the same analogy as before it could be:
>
> ```
> if line[-2:-1] == [r'\r']:
>
> ```

Running the following test:

----------------
line = b'abc\r\n'

forms = {
    "line[-2]":     line[-2],
    "line[-2:-1]":  line[-2:-1],
    "13":           13,
    "r'\\r'":       r'\r',
    'b"\\r"':       b'\r',
    "[r'\\r']":     [r'\r'],
}

for expr, val in forms.items():
    print(f"{expr:12} = {val!r:12} type={type(val).__name__}")
-------------------

I get

-------------------
line[-2]     = 13           type=int
line[-2:-1]  = b'\r'        type=bytes
13           = 13           type=int
r'\r'        = '\\r'        type=str
b"\r"        = b'\r'        type=bytes
[r'\r']      = ['\\r']      type=list
-------------------

so maybe 

```
line[-2:-1] == b'\r'
```

is the correct and short way?

> This is correct in any case. :-)

Thanks for checking this José.

One lifecycle I'll try and beat you
in the Emoticons per mail statistics ;)

Udi
-- 
lyx-devel mailing list
[email protected]
https://lists.lyx.org/mailman/listinfo/lyx-devel