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

add ranked suggestions to add error #2985

Merged
merged 5 commits into from
Feb 14, 2022
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
14 changes: 7 additions & 7 deletions src/API.jl
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ function add(ctx::Context, pkgs::Vector{PackageSpec}; preserve::PreserveLevel=PR
project_deps_resolve!(ctx.env, pkgs)
registry_resolve!(ctx.registries, pkgs)
stdlib_resolve!(pkgs)
ensure_resolved(ctx.env.manifest, pkgs, registry=true)
ensure_resolved(ctx, ctx.env.manifest, pkgs, registry=true)

for pkg in pkgs
if Types.collides_with_project(ctx.env, pkg)
Expand Down Expand Up @@ -298,7 +298,7 @@ function rm(ctx::Context, pkgs::Vector{PackageSpec}; mode=PKGMODE_PROJECT, all_p

mode == PKGMODE_PROJECT && project_deps_resolve!(ctx.env, pkgs)
mode == PKGMODE_MANIFEST && manifest_resolve!(ctx.env.manifest, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)

Operations.rm(ctx, pkgs; mode)
return
Expand Down Expand Up @@ -336,7 +336,7 @@ function up(ctx::Context, pkgs::Vector{PackageSpec};
mode == PKGMODE_MANIFEST && manifest_resolve!(ctx.env.manifest, pkgs)
project_deps_resolve!(ctx.env, pkgs)
manifest_resolve!(ctx.env.manifest, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)
end
Operations.up(ctx, pkgs, level; skip_writing_project)
return
Expand Down Expand Up @@ -375,7 +375,7 @@ function pin(ctx::Context, pkgs::Vector{PackageSpec}; all_pkgs::Bool=false, kwar
end

project_deps_resolve!(ctx.env, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)
Operations.pin(ctx, pkgs)
return
end
Expand All @@ -401,7 +401,7 @@ function free(ctx::Context, pkgs::Vector{PackageSpec}; all_pkgs::Bool=false, kwa
end

manifest_resolve!(ctx.env.manifest, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)

Operations.free(ctx, pkgs; err_if_free = !all_pkgs)
return
Expand All @@ -426,7 +426,7 @@ function test(ctx::Context, pkgs::Vector{PackageSpec};
project_resolve!(ctx.env, pkgs)
project_deps_resolve!(ctx.env, pkgs)
manifest_resolve!(ctx.env.manifest, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)
end
Operations.test(
ctx,
Expand Down Expand Up @@ -1020,7 +1020,7 @@ function build(ctx::Context, pkgs::Vector{PackageSpec}; verbose=false, kwargs...
end
project_resolve!(ctx.env, pkgs)
manifest_resolve!(ctx.env.manifest, pkgs)
ensure_resolved(ctx.env.manifest, pkgs)
ensure_resolved(ctx, ctx.env.manifest, pkgs)
Operations.build(ctx, Set{UUID}(pkg.uuid for pkg in pkgs), verbose)
end

Expand Down
10 changes: 6 additions & 4 deletions src/Operations.jl
Original file line number Diff line number Diff line change
Expand Up @@ -980,7 +980,7 @@ function build_versions(ctx::Context, uuids::Set{UUID}; verbose=false)
build_project_preferences = Base.get_preferences()
end
else
build_project_override = gen_target_project(ctx.env, ctx.registries, pkg, source_path, "build")
build_project_override = gen_target_project(ctx, pkg, source_path, "build")
with_load_path([projectfile_path(source_path)]) do
build_project_preferences = Base.get_preferences()
end
Expand Down Expand Up @@ -1617,7 +1617,9 @@ function parse_REQUIRE(require_path::String)
end

# "targets" based test deps -> "test/Project.toml" based deps
function gen_target_project(env::EnvCache, registries::Vector{Registry.RegistryInstance}, pkg::PackageSpec, source_path::String, target::String)
function gen_target_project(ctx::Context, pkg::PackageSpec, source_path::String, target::String)
env = ctx.env
registries = ctx.registries
test_project = Types.Project()
if projectfile_path(source_path; strict=true) === nothing
# no project file, assuming this is an old REQUIRE package
Expand All @@ -1630,7 +1632,7 @@ function gen_target_project(env::EnvCache, registries::Vector{Registry.RegistryI
package_specs = [PackageSpec(name=pkg) for pkg in test_pkgs]
registry_resolve!(registries, package_specs)
stdlib_resolve!(package_specs)
ensure_resolved(env.manifest, package_specs, registry=true)
ensure_resolved(ctx, env.manifest, package_specs, registry=true)
for spec in package_specs
test_project.deps[spec.name] = spec.uuid
end
Expand Down Expand Up @@ -1710,7 +1712,7 @@ function test(ctx::Context, pkgs::Vector{PackageSpec};
test_project_preferences = Base.get_preferences()
end
else
test_project_override = gen_target_project(ctx.env, ctx.registries, pkg, source_path, "test")
test_project_override = gen_target_project(ctx, pkg, source_path, "test")
with_load_path(projectfile_path(source_path)) do
test_project_preferences = Base.get_preferences()
end
Expand Down
43 changes: 36 additions & 7 deletions src/Types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,7 @@ function set_repo_source_from_registry!(ctx, pkg)
Pkg.Operations.update_registries(ctx; force=false)
registry_resolve!(ctx.registries, pkg)
end
ensure_resolved(ctx.env.manifest, [pkg]; registry=true)
ensure_resolved(ctx, ctx.env.manifest, [pkg]; registry=true)
# We might have been given a name / uuid combo that does not have an entry in the registry
for reg in ctx.registries
regpkg = get(reg, pkg.uuid, nothing)
Expand Down Expand Up @@ -885,7 +885,7 @@ function stdlib_resolve!(pkgs::AbstractVector{PackageSpec})
end

# Ensure that all packages are fully resolved
function ensure_resolved(manifest::Manifest,
function ensure_resolved(ctx::Context, manifest::Manifest,
pkgs::AbstractVector{PackageSpec};
registry::Bool=false,)::Nothing
unresolved_uuids = Dict{String,Vector{UUID}}()
Expand All @@ -901,21 +901,29 @@ function ensure_resolved(manifest::Manifest,
push!(unresolved_names, pkg.uuid)
end
isempty(unresolved_uuids) && isempty(unresolved_names) && return
msg = sprint() do io
msg = sprint(context = ctx.io) do io
if !isempty(unresolved_uuids)
println(io, "The following package names could not be resolved:")
print(io, "The following package names could not be resolved:")
for (name, uuids) in sort!(collect(unresolved_uuids), by=lowercase ∘ first)
print(io, " * $name (")
print(io, "\n * $name (")
if length(uuids) == 0
what = ["project", "manifest"]
registry && push!(what, "registry")
print(io, "not found in ")
join(io, what, ", ", " or ")
print(io, ")")
all_names = available_names(ctx; manifest, include_registries = registry)
all_names_ranked, any_score_gt_zero = fuzzysort(name, all_names)
if any_score_gt_zero
println(io)
prefix = " Suggestions:"
printstyled(io, prefix, color = Base.info_color())
REPL.printmatches(io, name, all_names_ranked; cols = REPL._displaysize(ctx.io)[2] - length(prefix))
end
else
join(io, uuids, ", ", " or ")
print(io, " in manifest but not in project")
print(io, " in manifest but not in project)")
end
println(io, ")")
end
end
if !isempty(unresolved_names)
Expand All @@ -928,6 +936,27 @@ function ensure_resolved(manifest::Manifest,
pkgerror(msg)
end

# copied from REPL to efficiently expose if any score is >0
function fuzzysort(search::String, candidates::Vector{String})
scores = map(cand -> (REPL.fuzzyscore(search, cand), -Float64(REPL.levenshtein(search, cand))), candidates)
candidates[sortperm(scores)] |> reverse, any(s -> s[1] > 0, scores)
end
Comment on lines +940 to +943
Copy link
Contributor

Choose a reason for hiding this comment

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

from Slack- it would be better to use REPL's levsort instead (https://github.com/JuliaLang/julia/blob/f3e3554910a79fa38d5662d14abe04e137075e18/stdlib/REPL/src/docview.jl#L652-L660), which is what the REPL uses for corrections: https://github.com/JuliaLang/julia/blob/f3e3554910a79fa38d5662d14abe04e137075e18/stdlib/REPL/src/docview.jl#L700-L707.

This does a lot better on DifferentielEquations:

julia> levsort("DifferentielEquations", all)
3-element Vector{String}:
 "DifferentialEquations"
 "DifferenceEquations"
 "DifferentialEvolutionMCMC"

vs

julia> fuzzysort("DifferentielEquations", all)
(["DifferentialEvolutionMCMC", "DifferentialDynamicsModels", "DifferentiableStateSpaceModels", "DifferentialEquations", "SimpleDifferentialOperators", "DiscreteDifferentialGeometry", "DifferentialForms", "DifferentialDynamicProgramming", "DifferenceEquations", "AbstractDifferentiation"    "BenchmarkingEconomicEfficiency", "BayesianNonparametricStatistics", "ApproxFunOrthogonalPolynomials", "GAP_pkg_normalizinterface_jll", "ReinforcementLearningEnvironmentAtari", "MultivariateOrthogonalPolynomials", "BlackBoxOptimizationBenchmarking", "SemiclassicalOrthogonalPolynomials", "OpenTelemetryExporterOtlpProtoGrpc", "ReinforcementLearningEnvironmentClassicControl"], false)


function available_names(ctx::Context = Context(); manifest::Manifest = ctx.env.manifest, include_registries::Bool = true)
all_names = String[]
for (_, pkgentry) in manifest
push!(all_names, pkgentry.name)
end
if include_registries
for reg in ctx.registries
for (_, pkgentry) in reg.pkgs
push!(all_names, pkgentry.name)
end
end
end
return unique(all_names)
end

function registered_uuids(registries::Vector{Registry.RegistryInstance}, name::String)
uuids = Set{UUID}()
for reg in registries
Expand Down
16 changes: 15 additions & 1 deletion test/new.jl
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,20 @@ end
@test_throws PkgError(
"version specification invalid when tracking a repository: `0.5.0` specified for package `Example`"
) Pkg.add(name="Example", rev="master", version="0.5.0")
# Adding with a slight typo gives suggestions
try
Pkg.add("Examplle")
@test false # to fail if add doesn't error
catch err
@test err isa PkgError
@test occursin("The following package names could not be resolved:", err.msg)
@test occursin("Examplle (not found in project, manifest or registry)", err.msg)
@test occursin("Suggestions:", err.msg)
# @test occursin("Example", err.msg) # can't test this as each char in "Example" is individually colorized
end
@test_throws PkgError(
"name, UUID, URL, or filesystem path specification required when calling `add`"
) Pkg.add(Pkg.PackageSpec())
# Adding an unregistered package
@test_throws PkgError Pkg.add("ThisIsHopefullyRandom012856014925701382")
# Wrong UUID
Expand All @@ -428,7 +442,7 @@ end
isolate(loaded_depot=true) do; mktempdir() do tempdir
close(LibGit2.init(tempdir))
try Pkg.add(path=tempdir)
@assert false
@test false # to fail if add doesn't error
catch err
@test err isa PkgError
@test match(r"^invalid git HEAD", err.msg) !== nothing
Expand Down