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

Path with space completion fix for REPL #8838

Merged
merged 1 commit into from
Nov 23, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions base/REPLCompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ function complete_path(path::AbstractString, pos)
push!(matches, id ? file * (@windows? "\\\\" : "/") : file)
end
end
matches, (nextind(path, pos-sizeof(prefix))):pos, length(matches) > 0
matches = UTF8String[replace(s, r"\s", "\\ ") for s in matches]
return matches, nextind(path, pos - sizeof(prefix) - length(matchall(r" ", prefix))):pos, length(matches) > 0
end

function complete_methods(input::AbstractString)
Expand All @@ -148,7 +149,6 @@ end
include("latex_symbols.jl")

const non_identifier_chars = [" \t\n\r\"\\'`\$><=:;|&{}()[],+-*/?%^~"...]
const non_filename_chars = [(" \t\n\r\"'`@\$><=;|&{(" * (@unix?"\\" : "")) ...]
const whitespace_chars = [" \t\n\r"...]

# Aux function to detect whether we're right after a
Expand Down Expand Up @@ -187,12 +187,13 @@ function completions(string, pos)
partial = string[1:pos]
inc_tag = Base.incomplete_tag(parse(partial , raise=false))
if inc_tag in [:cmd, :string]
startpos = nextind(partial, rsearch(partial, non_filename_chars, pos))
m = match(r"[\t\n\r\"'`@\$><=;|&\{]| (?!\\)",reverse(partial))
startpos = length(partial)-(m == nothing ? 1 : m.offset) + 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ivarne, @tkelman I think I have made the same mistanke here it should properly be:
startpos = nextind(partial, nextind(partial, length(partial) - (m == nothing ? 1 : m.offset)))
It does not crash the REPL but it can't complete on "ß C:\\.
Sorry about the mess I made now I understand the difficulties in indexing in strings.

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't crash when it should, because of #7811.

Copy link
Member

Choose a reason for hiding this comment

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

Since m.offset is guaranteed to be the offset of an ASCII character (that's what you were searching for), and ASCII characters are always represented by a single code unit in UTF-xx, isn't length(partial) - (m == nothing ? 1 : m.offset) + 2 endof(s) - start + 1 nextind(s,endof(s)) - start okay here?

r = startpos:pos
paths, r, success = complete_path(string[r], pos)
paths, r, success = complete_path(unescape_string(string[r]), pos)
if inc_tag == :string &&
length(paths) == 1 && # Only close if there's a single choice,
!isdir(string[startpos:start(r)-1] * paths[1]) && # except if it's a directory
!isdir(unescape_string(string[startpos:start(r)-1] * paths[1])) && # except if it's a directory
(length(string) <= pos || string[pos+1] != '"') # or there's already a " at the cursor.
paths[1] *= "\""
end
Expand Down
19 changes: 18 additions & 1 deletion base/string.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,24 @@ macro mstr(s...); triplequoted(s...); end
## shell-like command parsing ##

function shell_parse(raw::AbstractString, interp::Bool)
s = strip(raw)
s = lstrip(raw)
#Strips the end but respects the space when the string endswith "\\ "
r = RevString(s)
i = start(r)
c_old = nothing
while !done(r,i)
c, j = next(r,i)
if c == '\\' && c_old == ' '
i -= 1
break
elseif !(c in _default_delims)
break
end
i = j
c_old = c
end
s = s[1:end-i+1]

last_parse = 0:-1
isempty(s) && return interp ? (Expr(:tuple,:()),last_parse) : ([],last_parse)

Expand Down
22 changes: 22 additions & 0 deletions test/replcompletions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,28 @@ c,r = test_scomplete("\$a")
@test s[r] == "Pk"
end

let #test that it can auto complete with spaces in file/path
path = tempdir()
space_folder = randstring() * " r"
dir = joinpath(path, space_folder)
dir_space = replace(space_folder, " ", "\\ ")
mkdir(dir)
cd(path) do
open(joinpath(space_folder, "space .file"),"w") do f
s = @windows? "rm $dir_space\\\\space" : "cd $dir_space/space"
c,r = test_scomplete(s)
@test r == endof(s)-4:endof(s)
@test "space\\ .file" in c

s = @windows? "cd(\"$dir_space\\\\space" : "cd(\"$dir_space/space"
c,r = test_complete(s)
@test r == endof(s)-4:endof(s)
@test "space\\ .file\"" in c
end
end
rm(dir, recursive=true)
end

@windows_only begin
tmp = tempname()
path = dirname(tmp)
Expand Down