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

feat: add reverse flag to list command #3705

Merged
merged 17 commits into from
Jan 15, 2025
25 changes: 23 additions & 2 deletions libmamba/src/api/list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ namespace mamba
{
bool full_name;
bool no_pip;
bool reverse;
};

struct formatted_pkg
Expand All @@ -35,6 +36,10 @@ namespace mamba
{
return a.name < b.name;
}
bool compare_reverse_alphabetically(const formatted_pkg& a, const formatted_pkg& b)
{
return a.name >= b.name;
}

std::string strip_from_filename_and_platform(
const std::string& full_str,
Expand Down Expand Up @@ -75,6 +80,7 @@ namespace mamba
regex = '^' + regex + '$';
}


jjerphan marked this conversation as resolved.
Show resolved Hide resolved
std::regex spec_pat(regex);
auto all_records = prefix_data.all_pkg_mgr_records();

Expand All @@ -87,7 +93,17 @@ namespace mamba
{
keys.push_back(pkg.first);
}
std::sort(keys.begin(), keys.end());
if (options.reverse)
{
std::sort(keys.begin(), keys.end(), [](const std::string& a, const std::string& b)
{
return a > b;
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
});
}
else
{
std::sort(keys.begin(), keys.end());
}

for (const auto& key : keys)
{
Expand Down Expand Up @@ -168,7 +184,11 @@ namespace mamba
}
}

std::sort(packages.begin(), packages.end(), compare_alphabetically);
auto comparable =
options.reverse ?
compare_reverse_alphabetically :
compare_alphabetically;
std::sort(packages.begin(), packages.end(), comparable);

// format and print table
printers::Table t({ "Name", "Version", "Build", "Channel" });
Expand Down Expand Up @@ -208,6 +228,7 @@ namespace mamba
detail::list_options options;
options.full_name = config.at("full_name").value<bool>();
options.no_pip = config.at("no_pip").value<bool>();
options.reverse =config.at("reverse").value<bool>();
jjerphan marked this conversation as resolved.
Show resolved Hide resolved

auto channel_context = ChannelContext::make_conda_compatible(config.context());
detail::list_packages(config.context(), regex, channel_context, std::move(options));
Expand Down
4 changes: 4 additions & 0 deletions micromamba/src/list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ init_list_parser(CLI::App* subcom, Configuration& config)
.description("Do not include pip-only installed packages."));
subcom->add_flag("--no-pip", no_pip.get_cli_config<bool>(), no_pip.description());

auto& reverse = config.insert(Configurable("reverse", false)
.group("cli")
.description("List installed packages in reverse order."));
subcom->add_flag("--reverse", reverse.get_cli_config<bool>(), reverse.description());
// TODO: implement this in libmamba/list.cpp
/*auto& canonical = config.insert(Configurable("canonical", false)
.group("cli")
Expand Down
39 changes: 35 additions & 4 deletions micromamba/tests/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,17 @@
from . import helpers


@pytest.mark.parametrize("reverse_flag", ["", "--reverse"])
@pytest.mark.parametrize("quiet_flag", ["", "-q", "--quiet"])
@pytest.mark.parametrize("env_selector", ["", "name", "prefix"])
@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
def test_list(tmp_home, tmp_root_prefix, tmp_env_name, tmp_xtensor_env, env_selector, quiet_flag):
def test_list(tmp_home, tmp_root_prefix, tmp_env_name, tmp_xtensor_env, env_selector, quiet_flag, reverse_flag):
Copy link
Member

Choose a reason for hiding this comment

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

I know this is maybe beyond this PR because it wasn't done in the first place, but we should test the non --json case as well (you could have missed adding the corresponding std::sort, and no test would have pointed that...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a test for the non --json case, but the part checking "conda-forge" is assuming that "res" always has the same format.

Copy link
Member

Choose a reason for hiding this comment

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

This handling of flags might be useful to boil down the two tests into a single one handling all the flags.

flags = filter(None, [json_flag])
output = helpers.run_env("export", "-n", empty_env, *flags)

Copy link
Member

Choose a reason for hiding this comment

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

We discussed that IRL, let's forget about my last suggestion (it would make things more complicated than necessary).

if env_selector == "prefix":
res = helpers.umamba_list("-p", tmp_xtensor_env, "--json", quiet_flag)
res = helpers.umamba_list("-p", tmp_xtensor_env, "--json", quiet_flag, reverse_flag)
elif env_selector == "name":
res = helpers.umamba_list("-n", tmp_env_name, "--json", quiet_flag)
res = helpers.umamba_list("-n", tmp_env_name, "--json", quiet_flag, reverse_flag)
else:
res = helpers.umamba_list("--json", quiet_flag)
res = helpers.umamba_list("--json", quiet_flag, reverse_flag)

assert len(res) > 2

Expand All @@ -28,6 +29,36 @@ def test_list(tmp_home, tmp_root_prefix, tmp_env_name, tmp_xtensor_env, env_sele
for i in res
)

if reverse_flag == "--reverse":
assert names.index("xtensor") > names.index("xtl")
jjerphan marked this conversation as resolved.
Show resolved Hide resolved
else:
assert names.index("xtensor") < names.index("xtl")


@pytest.mark.parametrize("reverse_flag", ["", "--reverse"])
@pytest.mark.parametrize("quiet_flag", ["", "-q", "--quiet"])
@pytest.mark.parametrize("env_selector", ["", "name", "prefix"])
@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
def test_list_no_json(tmp_home, tmp_root_prefix, tmp_env_name, tmp_xtensor_env, env_selector, quiet_flag, reverse_flag):
if env_selector == "prefix":
res = helpers.umamba_list("-p", tmp_xtensor_env, quiet_flag, reverse_flag)
elif env_selector == "name":
res = helpers.umamba_list("-n", tmp_env_name, quiet_flag, reverse_flag)
else:
res = helpers.umamba_list(quiet_flag, reverse_flag)

assert len(res) > 10

assert "xtensor" in res
assert "xtl" in res

assert len(res[res.rindex("\u2500"):].split("\n")) - 2 == res.count("conda-forge")
jjerphan marked this conversation as resolved.
Show resolved Hide resolved

if reverse_flag == "--reverse":
assert res.find("xtensor") > res.find("xtl")
else:
assert res.find("xtensor") < res.find("xtl")


@pytest.mark.parametrize("quiet_flag", ["", "-q", "--quiet"])
@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True)
Expand Down
Loading