tomboy-notes / tomboy-ng

Next generation of Tomboy
MIT License
389 stars 38 forks source link

Note export does not handle "illegal" filename characters #217

Closed aguador closed 3 years ago

aguador commented 3 years ago

OK, this was a chance discovery. I happen to have a note on my Linux system in which I placed a "/" as a character in the title. The problem was that when attempting to export it (no matter plain text, rtf ,md) the "/" is, of course, interpreted as part of a filename and an error is returned saying the directory does not exist -- as, indeed, it does not in this case.

I can imagine four ways to address this:

Prohibiting the use of illegal filename characters in note titles seems an unnecessary inconvenience -- and, of course, such characters would have to include unacceptable characters across platforms (e.g., Linux accepts " \ " in a filename, but not Windows -- which would presumably accept "/").

Deleting characters that are not allowed in filenames is what currently happens with spaces, and would work and could, presumably be implemented easily. However, deleting such characters produces a filename that is less easily read by humans.

Allowing filename choice sounds great, but I am not sure of the coding issues involved. Besides a warning that the potential filename included one of a list of illegal characters, there would have to be a way to edit the ng-generated filename. Furthermore, while less convenient, users can always edit filenames after the fact should they so desire.

Substituting a "legal" character for one not allowed in a filename may be the most efficient. ng already id's and deletes spaces, so why not identify the other illegal characters and use "_" to substitute for those characters (including spaces)? That would imporove readibility of filenames for humans. The only potential downside here is filename length limitations, although that should be a rare case.

As an example of the problem, let'ts say I decide I want to create a series of notes that share a common stem, such as: "MaxFilenameLength-ABC", "MaxFilenameLength-DEF", etc. Since the stem "MaxFilenameLength" already uses up the number of allowed characters, the ng-generated filename would presumably truncate to the same name and, if the user were not attention to the warnings, overwrite each other. This seems a highly unlike case, but if needed might be solved by limiting the number of characters to "Max-X" and generating sequential numbers to be appended to .the "Max-X" name.

aguador commented 3 years ago

In the last paragraph that should be "if the user were not paying attention".

With the third option, perhaps there is a simple solution: trigger the system file save dialog which would allow the usere to edit the filename directly, presumably without greatly increasing the amount of ng code. What I do not know is if implementing that cross-platform presents special challenges.

davidbannon commented 3 years ago

Hmm, good catch. I have a note in my test collection with a title full of forward and reverse slashes but I have never tried to export it (apparently). I think a simple "delete bad char" model is appropriate, you second model above. I will take out both forward on back in both Unix and Windows, nobody wants a escape character in a file name either !

Thanks for reporting.

Davo

davidbannon commented 3 years ago

OK, I have replaced forward and back slashes and asterisk, all with an underscore. Easy and unobtrusive solution and one that probably works for almost all situations.

Thanks

davidbannon commented 3 years ago

Pre-release version now available that addressed this issue. Please see https://github.com/tomboy-notes/tomboy-ng/releases/tag/v0.31 If you do use it, please watch for anything unexpected and please report what you find, good or bad.

aguador commented 3 years ago

Oops, that is 0.31f, righ? Just did on test with a forward slash, and it was not replaced! Exporting works fine with Md with that intermediates step, but fails with plain txt and RTF.

davidbannon commented 3 years ago

Hmm, I have not checked but its possible I made my change on just the MD exporter. Will follow up. At present, I replace /, \ and *with _ but just remove spaces.

davidbannon commented 3 years ago

OK, now applies to all three export formats. I change forward and backward slashes, asterisk and a dot. The dot is not a bad char but looks like the start of an extension if displayed before the extension has been added. I still just delete a space, not put a underscore in, IMHO, its more readable in CamelCase but as a (wantabe) programmer, I am used to CamelCase. Be dead easy to change that.

Overall, the export is a mess, its quite inconsistent between MD and the other two, and, I want to change the whole MD export to conform with CommonMark so, work is needed. But issues raised by this ticket appear to be fixed.

Davo

aguador commented 3 years ago

Fantastic, looking forward to the next release (not that I use export that much!).

davidbannon commented 3 years ago

And this one also fixed in 0.32, quite easily !

Davo