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

peco not installed #6

Open
ghost opened this issue Sep 15, 2019 · 11 comments · May be fixed by #10
Open

peco not installed #6

ghost opened this issue Sep 15, 2019 · 11 comments · May be fixed by #10

Comments

@ghost
Copy link

ghost commented Sep 15, 2019

I tried:

] add InteractiveCodeSearch
using InteractiveCodeSearch
@search show

I get the following error:

┌ Warning: Matcher peco not installed.
│ See https://github.com/peco/peco for how to install peco.
│
└ @ InteractiveCodeSearch C:\Users\phg\.julia\packages\InteractiveCodeSearch\bvRfv\src\InteractiveCodeSearch.jl:559
ERROR: IOError: could not spawn `peco`: no such file or directory (ENOENT)

How do I install peco?
Is there a way for peco to be installed automatically together with the package?

@tkf
Copy link
Owner

tkf commented Sep 15, 2019

How do I install peco?

As mentioned in https://github.com/peco/peco#installation, just download it from https://github.com/peco/peco/releases and place it to somewhere in your PATH.

Is there a way for peco to be installed automatically together with the package?

It is possible, maybe by using https://github.com/JuliaPackaging/BinaryProvider.jl. I'm not planning to do this right now but if you or somebody get this work I'd be happy to merge the PR.

@rapus95
Copy link

rapus95 commented Jan 9, 2020

Ran into the same ^^ Since the Artifact system arrived, this might be even easier. :D
I might eventually give it a shot if I find time 😅

@tkf
Copy link
Owner

tkf commented Jan 9, 2020

Thanks, that would be great!

@tkf tkf changed the title Package does not work peco not installed Jan 9, 2020
@rapus95
Copy link

rapus95 commented Jan 10, 2020

I guess you want to keep Julia LTS compat, thus, even if I can make an artifacts solution, I have now clue how to backport that for pre 1.3 🤔

@tkf
Copy link
Owner

tkf commented Jan 10, 2020

I don't know much about the Artifacts system but maybe something like this would work?

@static if VERSION < v"1.3"
    const peco_artifact = nothing
else
    using Pkg.Artifacts
    const peco_artifact = artifact"peco"
end

I think it's OK to throw a warning/error in old julias.

@rapus95
Copy link

rapus95 commented Jan 10, 2020

Btw, when searching, where to put that code to, I found that part:

@static if VERSION < v"0.7-"
[...]

You might want to consider dropping all that pre 1.0 stuff similar to Compat's decision JuliaLang/Compat.jl#665

@tkf
Copy link
Owner

tkf commented Jan 10, 2020

Unlike Compat.jl there hasn't been much update in InteractiveCodeSearch.jl after julia 1.0 was out. So that's why it still supports 0.6. But yes, it's better be dropped now.

@rapus95
Copy link

rapus95 commented Jan 14, 2020

Finally after just too much time for that amount of code 😄

#10

Though, on julia pre 1.3 the package is somewhat unnecessary (but shouldn't fail is its not used)... But I don't know of conditional packages

@rapus95 rapus95 linked a pull request Jan 14, 2020 that will close this issue
@tkf
Copy link
Owner

tkf commented Jan 15, 2020

Regarding JuliaPackaging/Yggdrasil#377, do we need to keep updating P/peco/build_tarballs.jl to track peco releases? Maybe it's OK. I'm just wondering how it'd work.

@tkf
Copy link
Owner

tkf commented Jan 15, 2020

So, from the discussion in slack, it looks like we need to design an interface to make CONFIG.interactive_matcher more generic.

Option 1: (::IO) -> ::String

A straightforward option is to just factor out current use of run_matcher:

# Internal function to be used from `_choose_method`:
_run_matcher(input) = run_matcher(CONFIG.interactive_matcher, input)

# Let's still allow `CONFIG.interactive_matcher :: Cmd`
function run_matcher(matcher::Base.AbstractCmd, input::IO)
    maybe_warn_matcher(matcher)
    return String(read_stdout(input, matcher))
end

# Otherwise assume that it's a callable/function
run_matcher(matcher, input::IO) = matcher(input)

Option 2: (::Iterator{String}) -> ::String

Maybe option 1 is too low-level? I think a nicer abstraction is to pass an iterator of strings.

# `strings` is an iterable
_run_matcher(strings) = run_matcher(CONFIG.interactive_matcher, strings)

function run_matcher(matcher::Base.AbstractCmd, strings)
    maybe_warn_matcher(matcher)
    return read_stdout(matcher) do io
        for line in strings
            println(io, line)
        end
    end
end

run_matcher(matcher, strings) = matcher(strings)

(This creates a lot of intermediate Strings. But if we want performance afterwards, we can always create an AbstractString subtype that is non-allocating and overloads print(::IO, ::CustomStringType).)

In both cases, I think we can use peco_jll via:

@static if VERSION < v"1.3"
    import peco_jll

    peco(input) = peco_jll.peco() do cmd
        run_matcher(cmd, input)
    end
else
    peco(input) = run_matcher(`peco`, input)
end

I'm inclined to Option 2. What do you think? Is there a better way to do this?

@dpo
Copy link

dpo commented Sep 17, 2020

Have a look at how Ipopt.jl does it. They have a build.jl that installs the binaries for Julia < 1.3 and relies on the artifact system otherwise. It would be great if peco were installed automagically.

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 a pull request may close this issue.

3 participants