search

crystal-lang / shards

Dependency manager for the Crystal language
Other
765 stars 102 forks source link

added support for tilde-initial paths #309

Closed masukomi closed 2 years ago

masukomi commented 4 years ago

I.e. specifying the home directory by starting the path with a tilde.

Additionally added extra info to the error message when a shard.yml file isn't found.

This fixes Issue #308

All tests pass. No new tests added to cover this because there don't appear to be ANY tests for this class.

straight-shoota commented 4 years ago

Please use Path#expand/File#expand_path to implement this.

ysbaddaden commented 4 years ago

Please not that #308 may not be considered a bug.

masukomi commented 4 years ago

Please use Path#expand/File#expand_path to implement this. - @straight-shoota

That was my first attempted solution. Technically i just replaced the calls to local_path with expand_local_path (better to use existing functionality) because it just does File.expand_path)

BUT doing that converted ~/workspace/bayes_classifier to /Users/masukomi/workspace/scuttlebutt/weekly_ssb_summarization/~/workspace/bayes_classifier

there are 2 problems with this:

/Users/masukomi/workspace/scuttlebutt/weekly_ssb_summarization/ is the directory i was in when i Ran it and has NOTHING to do with where it should be finding the path.

The ~ is still there.

using Path[local_path].expand(home: true).to_s seems to work.... (committed, rebased, pushed) BUT I'm seeing some other issues that i wasn't seeing before. I think it's just other stuff on my system that's unrelated. I would feel more confident if there were tests on this class.

masukomi commented 4 years ago

additional issues resolved. corrected. rebased pushed.

it just needed to be expanded in spec?(version) too

ysbaddaden commented 4 years ago

Note that I have nothing against Path, I just want the bugfix to be as minimal as possible :)

straight-shoota commented 2 years ago

This PR has been superseeded by #538 and #541