Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

removed setting pos to 0 on unwatch #28

Closed
wants to merge 1 commit into from
Closed

removed setting pos to 0 on unwatch #28

wants to merge 1 commit into from

Conversation

paul-em
Copy link

@paul-em paul-em commented Mar 25, 2015

I don't know if this is a wanted feature, but whenever i unwatch and watch again I get the whole file because of that, altough I never set frombeginning to true.

I don't know if this is a wanted feature, but whenever i unwatch and watch again I get the whole file because of that, altough I never set frombeginning to true.
@lucagrulla
Copy link
Owner

Hi,

Thanks for the pull request.
I understand the issue; can you give me an example of a scenario where you like to unwatch and then watch? I'm interested in understanding your use cases better.

Thanks.

@paul-em
Copy link
Author

paul-em commented Mar 27, 2015

Hi!
I created a logfile reader dashboard that uses node-tail - check it out here. However I only need to watch the file as soon as someone connects to the dashboard.
So I initialize it and unwatch it right away, as soon as someone connects I watch again and unwatch on disconnect. here in the code

My current solution is to save the pos and reset it.

In which scenario do you see that it makes sense to leave it as is?

@lucagrulla
Copy link
Owner

Cool.
Unwatch is mainly there to allow resource cleaning(i.e. closing the watcher), I haven't thought about the scenario where you stop and restart without creating a new Tail object. Based on your use case, isn't @pos not correct anyway? if you unwatch now and watch again in 5 minutes time, you might not care of what happened in the past 5 minutes but really you want to start tailing from now, i.e.

@pos = stats.size

as it happens in the constructor.

In that case it might be worthwhile to move that line from being in the constructor to be in the watch method so that @pos is initialized properly at every watch call.

Something like:

watch: ->
    return if @isWatching
    @isWatching = true
    @pos = if @frombeginning then 0 else stats.size #moved here from the constructor 
    if fs.watch then @watcher = fs.watch @filename, @fsWatchOptions, (e) => @watchEvent e
    else
      fs.watchFile @filename, @fsWatchOptions, (curr, prev) => @watchFileEvent curr, prev

Any thoughts?

@lucagrulla
Copy link
Owner

I've been keen to make a general improvements to this specific part for a while but I've never found the time. I ended up implementing what done in this PR; I didn't merge the PR because it was simpler to implement straight into master than dealing with conflicts but the outcome is the same (see 5a858e6)

@lucagrulla lucagrulla closed this Dec 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants