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.

Sign up for GitHub

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

Jump to bottom

fix: Improve cleanPath platform compatability #249

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: Improve cleanPath platform compatability #249

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76e18ce
fix: Improve `cleanPath` platform compatability
hyperupcall Oct 12, 2023
738f1cc
fix: Use of `path`
hyperupcall Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions doctoc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@

var path = require('path')
, fs = require('fs')
, os = require('os')
, minimist = require('minimist')
, file = require('./lib/file')
, transform = require('./lib/transform')
, files;

function cleanPath(path) {
var homeExpanded = (path.indexOf('~') === 0) ? process.env.HOME + path.substr(1) : path;
function cleanPath(filepath) {
var homeExpanded = (filepath.indexOf('~') === 0) ? path.join(os.homedir(), filepath.substr(1)) : filepath;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is renaming path to filepath a mistake because it's unrelated to the topic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope it's intentional; I renamed path to filepath because the path parameter in the cleanPath function was shadowing the path variable defined at the top of the file (var path = require('path')). And this caused a bug, since path.join is a function exported from the path module - String.prototype.join() does not exist. A mistake in my testing prevented me from catching this earlier

As I understand both files and folders can be passed to the cleanPath function, maybe you prefer a different rename?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, okay, I thought it was about the rewrite of process.env.HOME only, will take a deeper look at it.


// Escape all spaces
return homeExpanded.replace(/\s/g, '\\ ');
Expand Down
Loading