Prompt user for missing -r and -e flags in pip install (#1180)

## Summary

If the user runs a command like `pip install requirements.txt`, we now
prompt them to ask if they meant to include the `-r` flag:

![Screenshot 2024-01-29 at 8 38
29 PM](82b9f7a2-2526-4144-b200-a5015e5b8a4b)

![Screenshot 2024-01-29 at 8 38
33 PM](bd8ebb51-2537-4540-a0e0-718e66a1c69c)

The specific logic is: if the requirement ends in `.txt` or `.in`, and
the file exists locally, prompt the user for `-r`. If the requirement
contains a directory separator, and the directory exists locally, prompt
the user for `-e`.

Closes #1166.
This commit is contained in:
Charlie Marsh 2024-01-30 10:58:45 -08:00 committed by GitHub
parent 7a937e0f60
commit aa3b79ec63
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 125 additions and 15 deletions

View file

@ -47,6 +47,8 @@ anstream = { workspace = true }
anyhow = { workspace = true }
chrono = { workspace = true }
clap = { workspace = true, features = ["derive"] }
console = { workspace = true }
dunce = { workspace = true }
fs-err = { workspace = true, features = ["tokio"] }
futures = { workspace = true }
indicatif = { workspace = true }
@ -68,7 +70,6 @@ tracing-tree = { workspace = true }
url = { workspace = true }
waitmap = { workspace = true }
which = { workspace = true }
dunce = "1.0.4"
[target.'cfg(target_os = "windows")'.dependencies]
mimalloc = "0.1.39"

View file

@ -118,7 +118,7 @@ pub(crate) async fn pip_compile(
.filter(|_| !upgrade.is_all())
.filter(|output_file| output_file.exists())
.map(Path::to_path_buf)
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.as_ref()
.map(|source| RequirementsSpecification::from_source(source, &extras))
.transpose()?

View file

@ -0,0 +1,52 @@
use anyhow::Result;
use console::{style, Key, Term};
/// Prompt the user for confirmation in the given [`Term`].
///
/// This is a slimmed-down version of [`dialoguer::Confirm`], with the post-confirmation report
/// enabled.
pub(crate) fn confirm(message: &str, term: &Term, default: bool) -> Result<bool> {
let prompt = format!(
"{} {} {} {} {}",
style("?".to_string()).for_stderr().yellow(),
style(message).for_stderr().white().bold(),
style("[y/n]").for_stderr().black().bright(),
style("").for_stderr().black().bright(),
style(if default { "yes" } else { "no" })
.for_stderr()
.cyan(),
);
term.write_str(&prompt)?;
term.hide_cursor()?;
term.flush()?;
// Match continuously on every keystroke, and do not wait for user to hit the
// `Enter` key.
let response = loop {
let input = term.read_key()?;
match input {
Key::Char('y' | 'Y') => break true,
Key::Char('n' | 'N') => break false,
Key::Enter => break default,
_ => {}
};
};
let report = format!(
"{} {} {} {}",
style("".to_string()).for_stderr().green(),
style(message).for_stderr().white().bold(),
style("·").for_stderr().black().bright(),
style(if response { "yes" } else { "no" })
.for_stderr()
.cyan(),
);
term.clear_line()?;
term.write_line(&report)?;
term.show_cursor()?;
term.flush()?;
Ok(response)
}

View file

@ -40,6 +40,7 @@ static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
mod commands;
mod compat;
mod confirm;
mod logging;
mod printer;
mod requirements;
@ -670,17 +671,17 @@ async fn run() -> Result<ExitStatus> {
let requirements = args
.src_file
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let constraints = args
.constraint
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let overrides = args
.r#override
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let index_urls = IndexLocations::from_args(
args.index_url,
@ -739,7 +740,7 @@ async fn run() -> Result<ExitStatus> {
let sources = args
.src_file
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let reinstall = Reinstall::from_args(args.reinstall, args.reinstall_package);
let no_binary = NoBinary::from_args(args.no_binary, args.no_binary_package);
@ -768,19 +769,23 @@ async fn run() -> Result<ExitStatus> {
let requirements = args
.package
.into_iter()
.map(RequirementsSource::Package)
.map(RequirementsSource::from_package)
.chain(args.editable.into_iter().map(RequirementsSource::Editable))
.chain(args.requirement.into_iter().map(RequirementsSource::from))
.chain(
args.requirement
.into_iter()
.map(RequirementsSource::from_path),
)
.collect::<Vec<_>>();
let constraints = args
.constraint
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let overrides = args
.r#override
.into_iter()
.map(RequirementsSource::from)
.map(RequirementsSource::from_path)
.collect::<Vec<_>>();
let index_urls = IndexLocations::from_args(
args.index_url,
@ -827,9 +832,13 @@ async fn run() -> Result<ExitStatus> {
let sources = args
.package
.into_iter()
.map(RequirementsSource::Package)
.map(RequirementsSource::from_package)
.chain(args.editable.into_iter().map(RequirementsSource::Editable))
.chain(args.requirement.into_iter().map(RequirementsSource::from))
.chain(
args.requirement
.into_iter()
.map(RequirementsSource::from_path),
)
.collect::<Vec<_>>();
commands::pip_uninstall(&sources, cache, printer).await
}

View file

@ -1,8 +1,9 @@
//! A standard interface for working with heterogeneous sources of requirements.
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use console::Term;
use fs_err as fs;
use rustc_hash::FxHashSet;
@ -12,6 +13,8 @@ use puffin_fs::NormalizedDisplay;
use puffin_normalize::{ExtraName, PackageName};
use requirements_txt::{EditableRequirement, FindLink, RequirementsTxt};
use crate::confirm;
#[derive(Debug)]
pub(crate) enum RequirementsSource {
/// A package was provided on the command line (e.g., `pip install flask`).
@ -24,14 +27,57 @@ pub(crate) enum RequirementsSource {
PyprojectToml(PathBuf),
}
impl From<PathBuf> for RequirementsSource {
fn from(path: PathBuf) -> Self {
impl RequirementsSource {
/// Parse a [`RequirementsSource`] from a [`PathBuf`].
pub(crate) fn from_path(path: PathBuf) -> Self {
if path.ends_with("pyproject.toml") {
Self::PyprojectToml(path)
} else {
Self::RequirementsTxt(path)
}
}
/// Parse a [`RequirementsSource`] from a user-provided string, assumed to be a package.
///
/// If the user provided a value that appears to be a `requirements.txt` file or a local
/// directory, prompt them to correct it (if the terminal is interactive).
pub(crate) fn from_package(name: String) -> Self {
// If the user provided a `requirements.txt` file without `-r` (as in
// `puffin pip install requirements.txt`), prompt them to correct it.
#[allow(clippy::case_sensitive_file_extension_comparisons)]
if name.ends_with(".txt") || name.ends_with(".in") {
if Path::new(&name).is_file() {
let term = Term::stderr();
if term.is_term() {
let prompt = format!(
"`{name}` looks like a requirements file but was passed as a package name. Did you mean `-r {name}`?"
);
let confirmation = confirm::confirm(&prompt, &term, true).unwrap();
if confirmation {
return Self::RequirementsTxt(name.into());
}
}
}
}
// If the user provided a path to a local directory without `-e` (as in
// `puffin pip install ../flask`), prompt them to correct it.
if name.contains('/') || name.contains('\\') {
if Path::new(&name).is_dir() {
let term = Term::stderr();
if term.is_term() {
let prompt =
format!("`{name}` looks like a local directory but was passed as a package name. Did you mean `-e {name}`?");
let confirmation = confirm::confirm(&prompt, &term, true).unwrap();
if confirmation {
return Self::RequirementsTxt(name.into());
}
}
}
}
Self::Package(name)
}
}
#[derive(Debug, Default, Clone)]