search

sysread / Reddit-API

Reddit API for perl
22 stars 11 forks source link

expand unix tilde symbol to find session file in home directory #9

Closed aggrolite closed 9 years ago

aggrolite commented 9 years ago

Running on OSX I provided ~/.reddit as the session file path, but Perl's open function cannot locate the file. Replacing ~ with $ENV{HOME} before opening the file fixes this issue.

I've tested this change on OSX and Debian Linux.

Another solution is using File::HomeDir

dthvt commented 9 years ago

Seems like a reasonable solution except it breaks Windows compatibility. I'm not a big fan of adding dependencies, but if File::HomeDir works across *NIX and Win platforms, I'd rather see that.

Also, I'd recommend making the path modifications prior to the debug statement or having a later debug statement show the modification, so that debugging doesn't mislead folks.

aggrolite commented 9 years ago

Seems like a reasonable solution except it breaks Windows compatibility.

I assumed Windows did not support the ~ shortcut. Am I wrong?

Ah...after some googling Powershell does indeed support ~. File::HomeDir then (or an alternative)? Or at the very least, removing the example filepath using ~ in the documentation's synopsis. I personally would like to use the ~ shortcut.

Also, I'd recommend making the path modifications prior to the debug statement or having a later debug statement show the modification, so that debugging doesn't mislead folks.

Noted, thanks. I will make the adjustment.

aggrolite commented 9 years ago

Apparently glob() can be used to expand the tilde. I've only tested this on my BSD install. No idea if it works on Windows. Also I have no clue of any corner cases.

aggrolite commented 9 years ago

Oh, check out File::Path::Expand. This looks like a lighter alternative to File::HomeDir

aggrolite commented 9 years ago

I've pushed a new commit which uses File::Path::Expand, which should work on Windows. This module does a search replace, but uses User::pwent when $ENV{HOME} does not exist. I don't have a Windows machine to test on, however.

Thoughts?

sysread commented 9 years ago

I'll be happy to add this. Let me do some testing with it first.

dthvt commented 9 years ago

Is there any harm in calling File::Path::Expand::expand_filename() on a $file_path without a ~? If not, I would recommend getting rid of the conditional check.

aggrolite commented 9 years ago

Looking at the source, File::Path::Expand simply does a search/replace on strings beginning with ~. So the regex check can be removed.

aggrolite commented 9 years ago

See my latest push. Should be acceptable assuming Windows tests okay.