jasongilman / proto-repl

A Clojure Development Environment package for the Atom editor
https://atom.io/packages/proto-repl
MIT License
563 stars 50 forks source link

open-file-containing-var broken on windows #217

Closed cemerick closed 7 years ago

cemerick commented 7 years ago

Given a source file (in the same project) at e.g. C:\foo\bar.clj, this code is yielding a string with a spurious leading slash (like /C:/foo/bar.clj), which windows (and atom) doesn't know what to do with.

When a source file is found in dependency jars, unzipping fails with:

decompressing /G:/.m2/repository/org/clojure/clojure/1.9.0-alpha9/clojure-1.9.0-alpha9.jar to G:\/.lein/tmp-atom-jars/org/clojure/clojure/1.9.0-alpha9/clojure-1.9.0-alpha9.jar
Error trying to open: IOException CreateProcess error=2, The system cannot find the file specified  java.lang.ProcessImpl.create (:-2)

What you want is to get the URL of the source ((.getResource (clojure.lang.RT/baseLoader) "clojure/core.clj")), and check its protocol ((.getProtocol source)); this will be "jar" for dependencies loaded from jars, "file" for files you can have atom load immediately after getting the platform-appropriate absolute path by converting the URL to a File.

(Sorry for not just creating a PR; (a) I'm completely green on atom, and (b) I'm in no position to test on linux or a mac, and don't want to break things that are working now.)

pedro-w commented 7 years ago

This is the same as (closed) issue #172. I had a go at writing a gist0 to explore some solutions. The problem I had was how to maintain a cache of JARs that have been expanded - it seems wasteful to unzip a potentially large file every time you want to access the definition of a variable within.

jasongilman commented 7 years ago

Yeah this code won't work the way it's written on windows. I'll see if I can work up a way that's cross platform compatible.

didibus commented 7 years ago

I submitted a pull-request which fixes this issue. I believe it should continue working on Mac and Windows, as I've used Java for all path manipulation and removed the dependency on the unzip external tool (its now all Clojure), but I have only tested it on Windows.

jasongilman commented 7 years ago

I tested this on my mac and it's working. Thanks for the fix, @didibus