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 args: --edit-page and --edit-patch #388

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions docs/src/usage.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Arguments:

Options:
-l, --list List all commands in the cache
--edit-page Edit custom page with `EDITOR`
--edit-patch Edit custom patch with `EDITOR`
-f, --render <FILE> Render a specific markdown file
-p, --platform <PLATFORM> Override the operating system, can be specified multiple times in order
of preference [possible values: linux, macos, sunos, windows, android,
Expand Down
8 changes: 8 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ pub(crate) struct Cli {
#[arg(short = 'l', long = "list")]
pub list: bool,

/// Edit custom page with `EDITOR`
#[arg(long, requires = "command")]
pub edit_page: bool,

/// Edit custom patch with `EDITOR`
#[arg(long, requires = "command", conflicts_with = "edit_page")]
pub edit_patch: bool,

/// Render a specific markdown file
#[arg(
short = 'f',
Expand Down
73 changes: 61 additions & 12 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,15 @@ compile_error!(

use std::{
env,
fs::create_dir_all,
io::{self, IsTerminal},
path::Path,
process,
process::Command,
};

use anyhow::anyhow;
use anyhow::Context;
use app_dirs::AppInfo;
use clap::Parser;

Expand Down Expand Up @@ -240,6 +245,32 @@ fn get_languages_from_env() -> Vec<String> {
)
}

fn spawn_editor(custom_pages_dir: &Path, file_name: &str) -> anyhow::Result<()> {
let res = create_dir_all(custom_pages_dir)
.context(format!("Fail to create dir {:?}", custom_pages_dir));
if !custom_pages_dir.is_dir() {
res?;
return Err(anyhow!("Fail to create dir {:?}", custom_pages_dir));
}
Comment on lines +249 to +254
Copy link
Collaborator

Choose a reason for hiding this comment

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

See #388 (comment)

Suggested change
let res = create_dir_all(custom_pages_dir)
.context(format!("Fail to create dir {:?}", custom_pages_dir));
if !custom_pages_dir.is_dir() {
res?;
return Err(anyhow!("Fail to create dir {:?}", custom_pages_dir));
}
create_dir_all(custom_pages_dir).context("Failed to create custom pages directory")?;


let custom_page_path = custom_pages_dir.join(file_name);
let Some(custom_page_path) = custom_page_path.to_str() else {
return Err(anyhow!("`custom_page_path.to_str()` failed"));
};
let Ok(editor) = env::var("EDITOR") else {
return Err(anyhow!(
"To edit a custom page, please set the `EDITOR` environment variable."
));
};
println!("Editing {custom_page_path:?}");

let status = Command::new(&editor).arg(custom_page_path).status()?;
if !status.success() {
return Err(anyhow!("{editor} exit with code {:?}", status.code()));
}
Ok(())
}

fn main() {
// Initialize logger
init_log();
Expand Down Expand Up @@ -271,6 +302,34 @@ fn main() {
}
};

let custom_pages_dir = config
.directories
.custom_pages_dir
.as_ref()
.map(PathWithSource::path);

// Note: According to the TLDR client spec, page names must be transparently
// lowercased before lookup:
// https://github.com/tldr-pages/tldr/blob/main/CLIENT-SPECIFICATION.md#page-names
let command = args.command.join("-").to_lowercase();

if args.edit_patch || args.edit_page {
let file_name = if args.edit_patch {
format!("{command}.patch.md")
} else {
format!("{command}.page.md")
};

custom_pages_dir
.context("To edit custom pages/patches, please specify a custom pages directory.")
.and_then(|custom_pages_dir| spawn_editor(custom_pages_dir, &file_name))
.unwrap_or_else(|err| {
print_error(enable_styles, &err);
process::exit(1);
});
return;
}

// Show various paths
if args.show_paths {
show_paths(&config);
Expand Down Expand Up @@ -313,19 +372,14 @@ fn main() {

// Check cache presence and freshness
if !cache_updated
&& (args.list || !args.command.is_empty())
&& (args.list || !command.is_empty())
&& check_cache(&cache, &args, enable_styles) == CheckCacheResult::CacheMissing
{
process::exit(1);
}

// List cached commands and exit
if args.list {
let custom_pages_dir = config
.directories
.custom_pages_dir
.as_ref()
.map(PathWithSource::path);
println!(
"{}",
cache.list_pages(custom_pages_dir, &platforms).join("\n")
Expand All @@ -334,12 +388,7 @@ fn main() {
}

// Show command from cache
if !args.command.is_empty() {
// Note: According to the TLDR client spec, page names must be transparently
// lowercased before lookup:
// https://github.com/tldr-pages/tldr/blob/main/CLIENT-SPECIFICATION.md#page-names
let command = args.command.join("-").to_lowercase();

if !command.is_empty() {
// Collect languages
let languages = args
.language
Expand Down
64 changes: 64 additions & 0 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,3 +990,67 @@ fn test_raw_render_file() {
.success()
.stdout(diff(include_str!("cache/pages/common/inkscape-v1.md")));
}

fn touch_custom_page(testenv: &TestEnv) {
let args = vec!["--edit-page", "foo"];

testenv
.command()
.args(&args)
.env("EDITOR", "touch")
.assert()
.success();
assert!(testenv.custom_pages_dir.path().join("foo.page.md").exists());
}

fn touch_custom_patch(testenv: &TestEnv) {
let args = vec!["--edit-patch", "foo"];

testenv
.command()
.args(&args)
.env("EDITOR", "touch")
.assert()
.success();
assert!(testenv
.custom_pages_dir
.path()
.join("foo.patch.md")
.exists());
}

#[test]
fn test_edit_page() {
let testenv = TestEnv::new().write_custom_pages_config();
touch_custom_page(&testenv);
}

#[test]
fn test_edit_patch() {
let testenv = TestEnv::new().write_custom_pages_config();
touch_custom_patch(&testenv);
}

#[test]
fn test_recreate_dir() {
let testenv = TestEnv::new().write_custom_pages_config();
touch_custom_patch(&testenv);
touch_custom_page(&testenv);
}

#[test]
fn test_custom_pages_dir_is_not_dir() {
let testenv = TestEnv::new().write_custom_pages_config();
let _ = std::fs::remove_dir_all(testenv.custom_pages_dir.path());
let _ = File::create(testenv.custom_pages_dir.path()).unwrap();
Comment on lines +1044 to +1045
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this might be a problem, because the created file is not automatically deleted after the tests are finished. However, I think we should in general change our test environment to place everything in unified temporary directory instead of creating multiple ones. I will make a follow up issue, so we can leave it as is for now

assert!(testenv.custom_pages_dir.path().is_file());

let args = vec!["--edit-patch", "foo"];

testenv
.command()
.args(&args)
.env("EDITOR", "touch")
.assert()
.failure();
}