Support saving backup and autosave files in configurable directories

[LiberalArtist]

trafficstars

Modifies path-utils:generate-autosave-name and path-utils:generate-backup-name to respond to preferences stored under 'path-utils:autosave-dir and 'path-utils:backup-dir, respectively, which can be used to designate a directory for saving the corresponding automatically-generated files, rather than saving them in the same directory as the originals.

This functionality is inspired by Emacs' ability to be configured to do likewise with backup and autosave files.

[rfindler]

Thank you for working on this. A few comments/questions:

• why remove the unit? (I would have initialized the preference in private/main.rkt, if that's the reason)

• this exception handler isn't needed (and same for the other one nearby) afaict:

  (with-handlers ([exn:fail? (λ (e) #f)])
     (and v (bytes->path v))))

• please don't leave commented out code; tests should be in test a submodule or in the test suite (probably in the test suite)

• if you could arrange the commit so that there are minimal changes that achieve the new functionality and then a separate commit that reformats things to match modern Racket, that will help me in the case that there is a subtle bug lurking here that we find out about years later...

[LiberalArtist]

if you could arrange the commit so that there are minimal changes that achieve the new functionality and then a separate commit that reformats things to match modern Racket, that will help me in the case that there is a subtle bug lurking here that we find out about years later...

I will do this now.

why remove the unit? (I would have initialized the preference in private/main.rkt, if that's the reason)

Partially for that reason, partially because I just wasn't sure of the benefit of having the unit (is there one?).

this exception handler isn't needed (and same for the other one nearby) afaict:

Here I was unsure, because the docs say that, "The marshall function might be called with any value returned from read and it must not raise an error (although it can return arbitrary results if it gets bad input). This might happen when the preferences file becomes corrupted, or is edited by hand.", but I thought this might have meant to refer to unmarshall: I believe that is the one called with the value from the file. The handler around marshall can certainly be removed if its input can be guaranteed to satisfy (or/c #f path?); the one in unmarshall seems more necessary, as it could be given #"" or a byte-string with (platform-specific) illegal characters.

please don't leave commented out code; tests should be in test a submodule or in the test suite (probably in the test suite)

I will do this. This is forcing me to think about how to make my informal tests actually work with the directory-exists? check … :)

[rfindler]

The unit is for the rest of the framework. The framework also tries to guarantee that all of the preferences are initialized before any of the other code runs to make it easier to reason about ordering (i.e., you don't have to), which is why preference initialization goes in main.

re unmarshalling: yes, that's right, sorry. But I think a bytes? check is better.

I just now noticed that main.rkt was reindented. Please do not do that.

Also, I don't see handling for the case that the user-specified directory does not exist. This wasn't (really) needed before because the save file itself handled that. But now there needs to be a fallback so that the autosave file doesn't either silently fail to be created (or pop up an error message every N seconds when the autosave is tried).

[LiberalArtist]

I tried to make sure the directory exists by checking directory-exists? in valid-preference-value?. Should I also check some other way?

[rfindler]

The directory might not exist later when the auto save (or backup) file is being saved.

[rfindler]

Which makes me think of another problem: what happens if there are multiple files with the same name (but in different directories)?

There probably needs to be some kind of table saying which one is which. Which also means we probably need more care to support concurrency than is currently present.

On Sun, Nov 12, 2017 at 9:34 AM Robby Findler [email protected] wrote:

The directory might not exist later when the auto save (or backup) file is being saved.

[LiberalArtist]

If I understand the question properly, I think this is handled because (when there is a non-false preference value) the functions derive the filename from the complete path to the original file. It is conceivably possible to engineer a collision, but the Emacs configuration I'm inspired by seems to just ignore this without problems in practice.

[rfindler]

Oh, I see encode-as-path-element now, sorry for not checking for that first.

Under Windows, the path length limit seems to be 260 chars, which seems plausible enough that probably something should be done to deal with that. Have you considered hashing the path? (I'm not sure it would be better, but it seems worth considering.)

Also, instead of using path->bytes, it is probably better to use split-path (or explode-path) and then concatenate things with some sentinel in between. I don't know what can go wrong if that doesn't happen and maybe bytes->path-element handles any malfeasance that can happen, but the docs suggest that it is necessary.

[LiberalArtist]

Someone reminded me of this pull request at RacketCon. I'm writing this mostly to remind myself to look at it again when I get home. I don't immediately remember what the remaining issues were, but ideally I'd love to get at least this much done in time for 7.1, if perhaps not making a UI.