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

fix for #11305 "subl" option in interactiveutil.jl #11329

Closed
wants to merge 1 commit into from

Conversation

timema
Copy link
Contributor

@timema timema commented May 18, 2015

I think I figured out a solution to issue #11305 function edit seems to create bad path for sublime text option (i.e. "subl") in interactiveutil.jl

I think I figured it out if I change the path from:
ENV["JULIA_EDITOR"]="/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl"
ENV["JULIA_EDITOR"]="/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl"
edname stays as:
"/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl" and not "subl" as it should while shell_split(edpath) produces the correct:
"/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"

in order to fix it we need to change:
in 0.3.8 interactiveutil.jl line 34
from: elseif beginswith(edname, "subl")
to: elseif beginswith(edname, "subl") || endswith(string(edname),"subl")

and in the latest pre 0.4 from
in 0.3.8 interactiveutil.jl line 36
from: startswith(edname, "subl") || edname == "atom"
to: startswith(edname, "subl") || edname == "atom" || endswith(string(edname),"subl")

This is a pull request to change the code for the latest pre 0.4 I am not sure how to make a pull reqest to fix if for the latest 0.3 version.

I think I figured out a solution to issue JuliaLang#1305 function edit seems to create bad path for sublime text option (i.e. "subl") in interactiveutil.jl

I think I figured it out if I change the path from:
ENV["JULIA_EDITOR"]="/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl"
ENV["JULIA_EDITOR"]="/Applications/Sublime\\ Text.app/Contents/SharedSupport/bin/subl"
edname stays as: 
"/Applications/Sublime\\ Text.app/Contents/SharedSupport/bin/subl" and not "subl" as it should while shell_split(edpath) produces the correct:
"/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"

in order to fix it we need to change:
in 0.3.8 interactiveutil.jl line 34
from:   elseif beginswith(edname, "subl")
to:       elseif beginswith(edname, "subl") || endswith(string(edname),"subl")

and in the latest pre 0.4 from
in 0.3.8 interactiveutil.jl line 36
from:   startswith(edname, "subl") || edname == "atom"
to:      startswith(edname, "subl") || edname == "atom" || endswith(string(edname),"subl")


This is a pull request to change the code for the latest pre 0.4  I am not sure how to make a pull reqest to fix if for the latest 0.3 version.
@jayschwa
Copy link
Member

This seems to treat a symptom and not a cause. We should understand why edname isn't being stripped down to the basename (thus causing the existing condition to fail).

@timema
Copy link
Contributor Author

timema commented May 18, 2015

I think the primary reason is that your suggetion of:
ENV["JULIA_EDITOR"]="/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl"
is not seen as a realpath() in:
(for some reason the git editor is not displaying the double \ after Sublime above)
editor = get(ENV,"JULIA_EDITOR", get(ENV,"VISUAL", get(ENV,"EDITOR", default_editor)))
if ispath(editor)
if isreadable(editor)
edpath = realpath(editor)
edname = basename(edpath)
else
error("can't find "$editor"")
end
else
edpath = edname = editor
end

so ispath(editor) is false leading to: edpath = edname = editor so edpath is the full name and shell_split(edpath) collapsed to: "/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"

when I use my original:
ENV["JULIA_EDITOR"]="/Applications/Sublime\ Text.app/Contents/SharedSupport/bin/subl"
ispath(editor) is true and edname = "subl"
and edpath = "/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"

however shell_split(edpath) turns to:
2-element Array{String,1}:
"/Applications/Sublime"
"Text.app/Contents/SharedSupport/bin/subl"

and doesn't work. That is why I originally suggested changing:
spawn($(shell_split(edpath)) $file:$line)
to
spawn($edpath $file:$line)
which works fine.

@ihnorton ihnorton changed the title fix for #1305 "subl" option in interactiveutil.jl fix for #11305 "subl" option in interactiveutil.jl May 19, 2015
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.

3 participants