entr should not break on rename

[mlhetland]

I was puzzled for a while about why I couldn't get Revise.entr to work – every time I modified a file, it just quit. Then I realized it was because my editor (vim) uses two-stage saving, where it writes to a temp file and then moves that to overwrite the original. And I don't think that is a situation where entr should conclude that the file is no longer relevant; indeed, this works just fine with the originale entr command-line tool.

I'm not sure exactly what the behavior should be – e.g., if renamed is also true if the original file is renamed to something else (in which case we might not want to keep watching it), but at least for my use-case (editing with vim), the current behavior is not helpful.

[timholy]

Nice debugging. Perhaps try putting in a short sleep and see if that fixes it?

Code for entr is quite small so I suspect you'll have little trouble playing with it, but don't hesitate to ask if you have questions.

[mlhetland]

Indeed – it's still quite close to my version, so that should be fine. I started looking at it, but was (1) a bit uncertain about what the behavior should be, and (2) a bit uncertain about how to add the appropriate tests :-}

I'm not sure how a sleep would work, though. The returned event will still indicate that the file has been renamed, no? That's information we get from the Node.js library regardless?

As far as I can see, the event returned is identical, regardless of whether a new file overwrites the one we're tracking or if the tracked file is renamed to something else (i.e., it's a FileEvent(true, false, false) either way).

The behavior of entr is:

1. Trigger once initially, unconditionally (as opposed to Revise.entr – maybe something to consider);
2. If the file is overwritten, trigger as usual;
3. If the file is moved elsewhere, quit with an error.

I'm not sure what the right thing is in case 3. One possibility is to not do anything particular – we'd end up with an error once we try to keep tracking changes after it's gone, anyway, and the resulting IOError is clear enough.

So: My suggestion would actually be simply to the conditional break when ret.renamed is true.

Whether we should add a single unconditional triggering of the function before we look for changes, I don't know. It might be useful to follow the behavior of the original tool, perhaps. At least for my code, this is sensible behavior, because I generally use it to generate some output, and though I'd like to re-generate the output when my code and input files change, I usually also want an initial version of the output when I start. But if you want to indicate changes, maybe you don't want that…

[timholy]

I haven't looked carefully. The recommendation to add a sleep was motivated by the assumption that the rename is transient, and if you wait a little there will be a second event that does something different. Maybe not true?

Sorry, in meetings, can't look now.

[mlhetland]

As far as I can see, that's not the case. The change is the file being overwritten – which I guess is then signalled as a rename, even though it's technically a rename of a different file, and a deletion of the one we're watching. And from my (simple) experimentation, it seems like there is just one change event triggered – which is how it should be, as it only changes once.

My previous comment was a bit verbose, but simply put, I simply suggest removing the special-casing of ret.renamed (i.e., the conditional break). This would give us the same semantics as the command-line tool (including failing if the file disappears, which makes sense to me).

[mlhetland]

The same semantics, that is, once we've gotten going; the command-line tool also triggers once before it start watching for changes. However, it has the comand-line switch -p:

     -p      Postpone the first execution of the utility until a file is modi-
             fied.

Whether that should be the default, an option or unavailable behavior in Revise.entr, I don't know.

[timholy]

For the first change, I'd approve running the update whenever the status is changed or renamed. (Not for timedout.)

The second seems reasonable too. I'd do it via a keyword argument.

[mlhetland]

Okay, tested this out, now, and it works just fine – but it doesn't seem to solve my problem; when I save from either vim or emacs, I end up with two events, now. That is true even if I make ret.renamed a no-op. I guess maybe they perform some timestamp or file permission corrections or the like?

However: The entr command-line tool also has this behavior (I just checked), so I guess that's the most natural thing to do. It does seem a bit wasteful to call the callback (at least) twice each time a file is saved, though. If we could do this more robustly, it might be a good idea to diverge from the command-line tool here.

In my code I added a keyword argument for the length of a pause to be taken after f() when triggered by a given file, until that file is triggered again (and after the initial call to f(), if postpone is false). Not sure if this is the "correct" way to do it, but I could send a pull request, if you'd like. (I haven't added any tests, though.)

[timholy]

A PR would be great. One way to flexibly handle those double-triggers is to set up a flag indicating that an update is scheduled, e.g.

const entr_update_scheduled = Ref(false)

# in entr body
    if !entr_update_scheduled[]
        entr_update_scheduled[] = true
        @async begin
            sleep(0.02)
            entr_update_scheduled[] = false
            revise()
            f()
        end
    end

With this it will wait 20ms before running the update, thereby giving editors a chance to finish making all their changes. Because we check entr_update_scheduled the update will only run once.

[mlhetland]

Seems like a good idea in principle, but it doesn't quite seem to do the trick. At least, I still end up with several calls (for some reason I got 3 calls, now). In my previous version, even a sleep of 1 second wasn't enough – though 2 worked. Because it was so long, I put it after the call to f—and I guess if the first thing that happens is changing the contents of the file, and the rest is just tweaking metadata, that shouldn't matter, but it's rather brittle, and quite inelegant, really.

Now, I just tried to repeatedly log the output of watch_file, and noticed a couple of things:

• When saving from vim, I first get a FileEvent(true, false, false) and then a FileEvent(false, true, false), which makes sense. Maybe this is a pattern occurring elsewhere, too, and which we may use.
• When saving from emacs, I first get a FileEvent(true, false, false), and then get an IOError, because the file no longer exists. I guess this is because emacs first renames the file with a ~ suffix, and then renames the saved file to the correct name (or, indeed, then writes that file to disk; haven't looked at which).

There may be several ways of reasonably using a small cluster of file system operations to modify a file, in cases where we only want to trigger once. Maybe the only solution is to have a moratorium after a call to f(), and to have it be user-customizable (as in my current code)? Having rather long breaks between each triggering might not be that absurd—after all, we have the same kind of pause (for different reasons) in poll_file, where the default is 5.007 seconds!

And I guess it has to be user-definable (if this is the way to go); in my current tests the callback is generally quite trivial, but if the callback itself takes a while to run, there is of course less of a problem anyway.

[timholy]

Since I basically never use entr, it will be best if others who do use it drive whatever change we make. I'll await a PR.

[mlhetland]

OK, I'll have a go.

[timholy]

Was this fixed by #308? If so please close.

[mlhetland]

Indeed, it was :-)