fix: sanitize names on file operations - #2546
Conversation
|
Fixed a bug where the status message color was stripped as well. Note that this will mostly be simplified by a followup refactor once it is merged along with the related PRs |
|
Added sanitation for history entries. With this PR merged, lf will handle all filenames with the same sanitation that is applied by |
|
@joelim-work Can we try to get this one merged for r42? |
joelim-work
left a comment
There was a problem hiding this comment.
The changes here look fine, but at this point I'd like to know what your plans for sanitization are. With this change there will now be three types of sanitization:
sanitizeName: The most strict version, does not allow control characterssanitizeForDisplay: Allows tabssanitizeMessage: Allows terminal sequences
I did a quick search of the codebase to see which type of sanitization is used for which feature:
| Feature | Sanitization Type |
|---|---|
| Default preview | sanitizeForDisplay |
| Sixel preview | sanitizeForDisplay |
| Directory view pane | sanitizeName |
| Prompt line | sanitizeName |
| Ruler | sanitizeName |
| Command line | sanitizeName |
| Menus | sanitizeName |
| Query output | sanitizeName |
| Echo | sanitizeNessamge |
Echoerr (expects plain string formatted by errorfmt) |
sanitizeName |
For a given feature, how do you decide which type of sanitization is required? Regarding future development, I can easily see the possibility where contributors either use the wrong type of sanitization or forget about sanitization altogether.
Thats exactly why I plan to refactor all sanitation code and have it in a separate source file along with simple instructions that describe which one is which. At the moment these functions are spread all over. |
joelim-work
left a comment
There was a problem hiding this comment.
OK I think the changes look fine now, thanks once again for the patch.
Addresses missing sanitation on filenames prompts like rename