Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

return path in linux, fixes #25 #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nkovacs
Copy link

@nkovacs nkovacs commented Jun 6, 2014

#25

@kevinsawicki kevinsawicki self-assigned this Jun 20, 2014
@kevinsawicki
Copy link
Contributor

Sorry for the delay in reviewing this, the specs appear to be failing on Travis, are they passing for you locally?

fullpath += e->name;
}

std::vector<char> path(fullpath.begin(), fullpath.end());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this path variable needed? Could fullpath be inserted into the handlemap instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're watching a directory, handlemap contains the path of the directory, and the inotify events give you just the name of the file, so you have to take the directory path from the handlemap and add the filename to it. You can't put fullpath back into handlemap, because that would then be the modified file's full path, and the next modified file would then get an invalid path.
The path variable is just because I need to convert it to a std::vector, at least that's what it originally was, so I kept it to be safe. I'm not very good at C++ though.

@nkovacs
Copy link
Author

nkovacs commented Jun 20, 2014

I did have to modify them, because the tests were expecting the incorrect behavior (empty strings).
The modified specs did pass for me, I'll take another look.

@nkovacs
Copy link
Author

nkovacs commented Jun 20, 2014

Here's the output:
https://gist.github.com/nkovacs/2c2120d77b79d8d5b896

More tests are skipped, but the three that failed on travis pass for me.

Edit: travis is on os x, and apparently the os x version also returns an empty string in some cases, which now fails with the modified tests.

@nkovacs
Copy link
Author

nkovacs commented Sep 19, 2014

Rebased. The tests pass on a linux host: https://travis-ci.org/nkovacs/node-pathwatcher/jobs/35714398, https://travis-ci.org/nkovacs/node-pathwatcher/jobs/35714399
Your tests expect the broken behavior ("when a watched path is changed, it fires the callback with the event type and empty path"), I fixed them, because the now fixed linux code did not pass, but the os x code is also broken, so now the tests fail with your travis config.

@kevinsawicki kevinsawicki removed their assignment Aug 6, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants