Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#22 config files fail if MSDOS line breaks

release
closed-postponed
Joachim Lous
Program (402)
2
2003-04-03
2001-03-24
Anonymous
No

If a config file has been saved with CR+LF line breaks
(as some of the contrib ones on the web site are), then
attempting to load them (using -import) causes them to
be rejected with spurious parser errors.

Made extra confusing if one tries to look for the error
using nedit on the config file, as the line breaks will
be hidden (usually good).

Fairly minor bug, but can be extremely frustrating if
one doesn't work out what is going wrong.

Discussion

  • Alexander Mai
    Alexander Mai
    2001-04-09

    • priority: 5 --> 1
     
  • Alexander Mai
    Alexander Mai
    2001-04-09

    Logged In: YES
    user_id=15180

    config files, etc. are supposed to be in a native
    text format. There is no need to handle foreign formats
    ecept the files which we want to edit IMHO.
    Other opinions?

     
  • Scott Tringali
    Scott Tringali
    2001-04-13

    Logged In: YES
    user_id=11321

    I can't imagine any harm in filtering out CRLF. People are
    quite likely to get patterns from the website, and they
    should sorta work...

     
  • Alexander Mai
    Alexander Mai
    2001-08-24

    Logged In: YES
    user_id=15180

    Ok, should go on TODO list for 5.3 ...

     
  • Alexander Mai
    Alexander Mai
    2001-08-24

    • labels: --> Program
    • priority: 1 --> 2
    • status: open --> open-postponed
     
  • Joachim Lous
    Joachim Lous
    2002-10-02

    Logged In: YES
    user_id=82866

    I have looked a bit at the code, and am considering fixing this
    by simply skipping any and all CR characters in the resource
    file. This would tolerate all types of line breaks without any
    prior detection of file format type. The only cost I can see
    woud be that literal 'cr's can not occur in the content, but as
    far as I can tell that will never be a problem with X resource
    files.

    Would this approach be acceptable?

     
  • Eddy De Greef
    Eddy De Greef
    2002-10-02

    Logged In: YES
    user_id=73597

    That looks fine with me.
    Note that there is a convertFromDosFileString() function in
    source/file.c. Maybe you can reuse it (after making it
    public and possibly moving it to one of the utils files).

    It doesn't solve the problem when the config file is in Mac
    format though (only CR for line break), but that is nearly
    unsolvable anyway (X routines read and parse the file directly).

     
  • Joachim Lous
    Joachim Lous
    2002-10-02

    Logged In: YES
    user_id=82866

    Ah, I thought mac was LF/CR. But in any case, the Xrm call
    in question here actually has a parallell that takes a string in
    stead of a file, so we can perfectly well buffer and filter it any
    way we like without too much effort.

    Thanks for the tips on existing code; I'll look into it.

     
  • Andrew Hood
    Andrew Hood
    2002-10-02

    Logged In: YES
    user_id=36856

    f you are thinking of using part of file.c why not use the
    whole formatOfFile() convertFromDosFileString()
    convertFromMacFileString() process?

     
  • Logged In: YES
    user_id=81393

    Actually I see no problem as long as only \r\n is converted
    into \n as suggested by Eddy (using
    convertFromDosFileString()).
    Don't forget about the word delimiters field
    (languageModeRec.delimiters). Theoretically it could contain
    \r (or even - highly improbable - \r\n). Aparrantly
    EscapeSensitiveChars() and ReadQuotedString() in should be
    adopted to quote \r, too.

     
  • Joachim Lous
    Joachim Lous
    2002-10-02

    Logged In: YES
    user_id=82866

    > Don't forget about the word delimiters field
    > (languageModeRec.delimiters). Theoretically it could
    > contain \r (or even - highly improbable - \r\n).

    Surely not in literal form in an X resource file?

     
  • Logged In: YES
    user_id=81393

    Of course ;-\ in literal (but enclosed in"")! Just verified it.
    You'll find something like

    TestLM:::::::"<np><cr>
    "

    there (<np> and <cr> beeing just one character, each) if one
    puts just the plain characters (invisibly) into the
    delimiters dialog field. (BTW, Putting \r in the field makes
    'r' an word delimiter character).

     
  • Joachim Lous
    Joachim Lous
    2002-10-02

    Logged In: YES
    user_id=82866

    OK. But since hearing about all the existing functions I plan
    to use them in stead anyway, so that should solve the
    problem (i.e. the files that won't work then are inherently fubar
    as text files in any case, and the problem should be solved
    by requiring escaping, not by exotic parsing algorithms).

     
  • Joachim Lous
    Joachim Lous
    2002-10-03

    Logged In: YES
    user_id=82866

    OK, the fix is ready. Should I post a patch, or just commit?

    It is a really straightforward fix: adds a public function char
    *ReadAnyTextFile(const char *fileName) to file.c, which uses
    existing file.c functions to detect linebreak type and convert if
    necessary. ImportPrefFile then uses it and
    XrmGetStringDatabase in stead of XrmGetFileDatabase as
    before.

     
  • Joachim Lous
    Joachim Lous
    2002-10-10

    Logged In: YES
    user_id=82866

    Fix posted as patch #621211. Someone close?

     
  • Joachim Lous
    Joachim Lous
    2003-03-28

    Logged In: YES
    user_id=82866

    Posted a new version of the patch (still under #621211), which now handles normal settings files, settings importing, and macro loading. I think that covers everything people are likely to get from the internet or edit by hand, and thus might possibly be in the wrong format.

    If someone can confirm it's OK, I'll commit it.

    Note that this is _not_ the new settings format I've been talking about, this is merely support for different newline types.

     
  • Thorsten Haude
    Thorsten Haude
    2003-03-28

    Logged In: YES
    user_id=119143

    I tried your first patch, and it worked. I just am not sure
    that this is a bug.

     
  • Eddy De Greef
    Eddy De Greef
    2003-03-28

    Logged In: YES
    user_id=73597

    I went through the new patch and it all looks ok to me.
    Please go ahead and commit it.

     
  • Eddy De Greef
    Eddy De Greef
    2003-03-28

    • assigned_to: nobody --> jlous
     
  • Joachim Lous
    Joachim Lous
    2003-04-03

    • status: open-postponed --> closed-postponed