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

use glob v5.0.3 to follow symlinks and return realpaths #586

Closed
wants to merge 2 commits into from
Closed

use glob v5.0.3 to follow symlinks and return realpaths #586

wants to merge 2 commits into from

Conversation

jldec
Copy link
Contributor

@jldec jldec commented Mar 13, 2015

The glob module now has options to return realpaths and follow symlinks. With glob v5.0.3. this feature is usable with node-inspector.

Preloaded scripts in symlinked directories now show up properly in the inspector source tree and breakpoints work, fixing issue #370.

f.w.i.w. i also added a little debug tracing in ScriptFileStorage. e.g. to trace during tests do:

DEBUG=node-inspector:ScriptFileStorage npm test

Fix #370

var relativePath = relativeUnixPath.split('/').join(path.sep);
return path.join(rootFolder, relativePath);
result = result.map(function(unixPath) {
return unixPath.split('/').join(path.sep);
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure the paths are returned as absolute, even though you specify rootFolder via options.cwd and not as options.root?

Related: 1c06f8a and isaacs/node-glob@f3105a3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes - i had to fix the code above to avoid prepending the rootfolder twice (which breaks inspector). i also had a little exchange with isaacs about the realpath returned paths being absolute now (and the match results not). isaacs/node-glob#184

@bajtos bajtos self-assigned this Apr 24, 2015
@bajtos
Copy link
Member

bajtos commented Apr 24, 2015

Thank you for the pull request, sorry for taking so long to review it.

Can you please rebase it on top of the current master and address my comment above?

@jldec
Copy link
Contributor Author

jldec commented Apr 24, 2015

rebased and comment addressed.
I also moved to the latest glob version which has updated absolute-path polyfill.
isaacs/node-glob@8552301
Hopefully this will improve things under windows. (i don't have a test setup for windows)

@bajtos bajtos closed this in 5e368e8 Apr 27, 2015
@bajtos
Copy link
Member

bajtos commented Apr 27, 2015

Landed via 5e368e8. I have improved the README text a bit, squashed the commits and added more details to the commit message.

Thank you for the contribution!

marcominetti pushed a commit to marcominetti/node-inspector that referenced this pull request May 19, 2015
Upgrade "glob" to v5.x and use the new options "follow" and "realpath".

This way the file paths returned to the UI are the same as those used
by Node/V8 runtime, which allows users to set breakpoints in symlinked
files.

Close node-inspector#586
Fix node-inspector#370
auchenberg pushed a commit to auchenberg/node-inspector that referenced this pull request Jun 27, 2015
Upgrade "glob" to v5.x and use the new options "follow" and "realpath".

This way the file paths returned to the UI are the same as those used
by Node/V8 runtime, which allows users to set breakpoints in symlinked
files.

Close node-inspector#586
Fix node-inspector#370
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.

Cannot set breakpoints in module loaded with npm link
2 participants