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

refactor: further separate CLI logic from the API related functionality (see #117) #124

Open
wants to merge 15 commits into
base: v3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 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
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ required-features = ["cli"]
annotate-snippets = {version = "^0.9.1", optional = true}
clap = {version = "^4.5.18", features = ["cargo", "derive", "env", "wrap_help"], optional = true}
clap_complete = {version = "^4.5.2", optional = true}
enum_dispatch = {version = "0.3.13", optional = true}
is-terminal = {version = "0.4.3", optional = true}
pulldown-cmark = {version = "0.10.2", optional = true}
reqwest = {version = "^0.11", default-features = false, features = ["json"]}
Expand All @@ -33,7 +34,7 @@ tokio = {version = "^1.0", features = ["macros"]}

[features]
annotate = ["dep:annotate-snippets"]
cli = ["annotate", "color", "dep:clap", "dep:is-terminal", "multithreaded"]
cli = ["annotate", "color", "dep:clap", "dep:enum_dispatch", "dep:is-terminal", "multithreaded"]
cli-complete = ["cli", "clap_complete"]
color = ["annotate-snippets?/color", "dep:termcolor"]
default = ["cli", "native-tls"]
Expand Down Expand Up @@ -61,7 +62,7 @@ license = "MIT"
name = "languagetool-rust"
readme = "README.md"
repository = "https://github.com/jeertmans/languagetool-rust"
rust-version = "1.74.0"
rust-version = "1.75.0"
version = "2.1.4"

[package.metadata.docs.rs]
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,14 @@ languagetool-rust = "^2.1"

```rust
use languagetool_rust::api::{check, server::ServerClient};
use std::borrow::Cow;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
let client = ServerClient::from_env_or_default();

let req = check::Request::default()
.with_text("Some phrase with a smal mistake".to_string()); // # codespell:ignore smal
.with_text(Cow::Borrowed("Some phrase with a smal mistake")); // # codespell:ignore smal

println!(
"{}",
Expand Down
8 changes: 5 additions & 3 deletions benches/benchmarks/check_texts.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::borrow::Cow;

use codspeed_criterion_compat::{criterion_group, Criterion, Throughput};
use futures::future::join_all;
use languagetool_rust::{
Expand Down Expand Up @@ -29,12 +31,12 @@ async fn request_until_success(req: &Request, client: &ServerClient) -> Response
}

#[tokio::main]
async fn check_text_basic(text: &str) -> Response {
async fn check_text_basic(text: &'static str) -> Response {
let client = ServerClient::from_env().expect(
"Please use a local server for benchmarking, and configure the environ variables to use \
it.",
);
let req = Request::default().with_text(text.to_string());
let req = Request::default().with_text(Cow::Borrowed(text));
request_until_success(&req, &client).await
}

Expand All @@ -48,7 +50,7 @@ async fn check_text_split(text: &str) -> Response {

let resps = join_all(lines.map(|line| {
async {
let req = Request::default().with_text(line.to_string());
let req = Request::default().with_text(Cow::Owned(line.to_string()));
let resp = request_until_success(&req, &client).await;
check::ResponseWithContext::new(req.get_text(), resp)
}
Expand Down
2 changes: 1 addition & 1 deletion rustfmt.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
condense_wildcard_suffixes = true
edition = "2021"
# error_on_line_overflow = true
# error_on_unformatted = true
force_multiline_blocks = true
Expand All @@ -9,5 +10,4 @@ imports_granularity = "Crate"
match_block_trailing_comma = true
normalize_doc_attributes = true
unstable_features = true
version = "Two"
wrap_comments = true
140 changes: 41 additions & 99 deletions src/api/check.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
//! Structures for `check` requests and responses.

#[cfg(feature = "cli")]
use std::path::PathBuf;
use std::{borrow::Cow, mem, ops::Deref};

#[cfg(feature = "annotate")]
use annotate_snippets::{
display_list::{DisplayList, FormatOptions},
snippet::{Annotation, AnnotationType, Slice, Snippet, SourceAnnotation},
};
#[cfg(feature = "cli")]
use clap::{Args, Parser, ValueEnum};
use clap::{Args, ValueEnum};
use serde::{Deserialize, Serialize, Serializer};

use crate::error::{Error, Result};

/// Requests
// REQUESTS

/// Parse `v` is valid language code.
///
Expand Down Expand Up @@ -396,7 +395,7 @@ pub struct Request {
clap(short = 't', long, conflicts_with = "data", allow_hyphen_values(true))
)]
#[serde(skip_serializing_if = "Option::is_none")]
pub text: Option<String>,
pub text: Option<Cow<'static, str>>,
/// The text to be checked, given as a JSON document that specifies what's
/// text and what's markup. This or 'text' is required.
///
Expand Down Expand Up @@ -431,7 +430,7 @@ pub struct Request {
/// will only be activated when you specify the variant, e.g. `en-GB`
/// instead of just `en`.
#[cfg_attr(
all(feature = "cli", feature = "cli", feature = "cli"),
feature = "cli",
clap(
short = 'l',
long,
Expand All @@ -448,8 +447,7 @@ pub struct Request {
)]
#[serde(skip_serializing_if = "Option::is_none")]
pub username: Option<String>,
/// Set to get Premium API access: [your API
/// key](https://languagetool.org/editor/settings/api).
/// Set to get Premium API access: your API key (see <https://languagetool.org/editor/settings/api>).
#[cfg_attr(
feature = "cli",
clap(short = 'k', long, requires = "username", env = "LANGUAGETOOL_API_KEY")
Expand Down Expand Up @@ -497,7 +495,7 @@ pub struct Request {
/// If true, only the rules and categories whose IDs are specified with
/// `enabledRules` or `enabledCategories` are enabled.
#[cfg_attr(feature = "cli", clap(long))]
#[serde(skip_serializing_if = "is_false")]
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub enabled_only: bool,
/// If set to `picky`, additional rules will be activated, i.e. rules that
/// you might only find useful when checking formal text.
Expand Down Expand Up @@ -531,15 +529,10 @@ impl Default for Request {
}
}

#[inline]
fn is_false(b: &bool) -> bool {
!(*b)
}

impl Request {
/// Set the text to be checked and remove potential data field.
#[must_use]
pub fn with_text(mut self, text: String) -> Self {
pub fn with_text(mut self, text: Cow<'static, str>) -> Self {
self.text = Some(text);
self.data = None;
self
Expand Down Expand Up @@ -572,7 +565,7 @@ impl Request {
///
/// If both `self.text` and `self.data` are [`None`].
/// If any data annotation does not contain text or markup.
pub fn try_get_text(&self) -> Result<String> {
pub fn try_get_text(&self) -> Result<Cow<'static, str>> {
if let Some(ref text) = self.text {
Ok(text.clone())
} else if let Some(ref data) = self.data {
Expand All @@ -588,7 +581,7 @@ impl Request {
));
}
}
Ok(text)
Ok(Cow::Owned(text))
} else {
Err(Error::InvalidRequest(
"missing either text or data field".to_string(),
Expand All @@ -604,7 +597,7 @@ impl Request {
/// If both `self.text` and `self.data` are [`None`].
/// If any data annotation does not contain text or markup.
#[must_use]
pub fn get_text(&self) -> String {
pub fn get_text(&self) -> Cow<'static, str> {
self.try_get_text().unwrap()
}

Expand All @@ -614,15 +607,20 @@ impl Request {
/// # Errors
///
/// If `self.text` is none.
pub fn try_split(&self, n: usize, pat: &str) -> Result<Vec<Self>> {
let text = self
.text
.as_ref()
.ok_or(Error::InvalidRequest("missing text field".to_string()))?;
pub fn try_split(mut self, n: usize, pat: &str) -> Result<Vec<Self>> {
let text = mem::take(&mut self.text)
Copy link
Owner

Choose a reason for hiding this comment

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

Here, one goal would be that try_split returns a reference to &self, so the signature should be as follows:

pub fn try_split<'source>(&'source self, n: usize, pat: &str) -> Result<Vec<Self<'source>>>

Note that we should not need to mutate the actual request, because we only take references.

.ok_or_else(|| Error::InvalidRequest("missing text field".to_string()))?;
let string: &str = match &text {
Cow::Owned(s) => s.as_str(),
Cow::Borrowed(s) => s,
};

Ok(split_len(text.as_str(), n, pat)
Ok(split_len(string, n, pat)
.iter()
.map(|text_fragment| self.clone().with_text(text_fragment.to_string()))
.map(|text_fragment| {
self.clone()
.with_text(Cow::Owned(text_fragment.to_string()))
})
.collect())
}

Expand All @@ -634,101 +632,34 @@ impl Request {
///
/// If `self.text` is none.
#[must_use]
pub fn split(&self, n: usize, pat: &str) -> Vec<Self> {
pub fn split(self, n: usize, pat: &str) -> Vec<Self> {
self.try_split(n, pat).unwrap()
}
}

/// Parse a string slice into a [`PathBuf`], and error if the file does not
/// exist.
#[cfg(feature = "cli")]
fn parse_filename(s: &str) -> Result<PathBuf> {
let path_buf: PathBuf = s.parse().unwrap();

if path_buf.is_file() {
Ok(path_buf)
} else {
Err(Error::InvalidFilename(s.to_string()))
}
}

/// Support file types.
#[cfg(feature = "cli")]
#[derive(Clone, Debug, Default, ValueEnum)]
#[non_exhaustive]
pub enum FileType {
/// Auto.
#[default]
Auto,
/// Markdown.
Markdown,
/// Typst.
Typst,
}

/// Check text using LanguageTool server.
///
/// The input can be one of the following:
///
/// - raw text, if `--text TEXT` is provided;
/// - annotated data, if `--data TEXT` is provided;
/// - raw text, if `-- [FILE]...` are provided. Note that some file types will
/// use a
/// - raw text, through stdin, if nothing is provided.
#[cfg(feature = "cli")]
#[derive(Debug, Parser)]
pub struct CheckCommand {
/// If present, raw JSON output will be printed instead of annotated text.
/// This has no effect if `--data` is used, because it is never
/// annotated.
#[cfg(feature = "cli")]
#[clap(short = 'r', long)]
pub raw: bool,
/// Sets the maximum number of characters before splitting.
#[clap(long, default_value_t = 1500)]
pub max_length: usize,
/// If text is too long, will split on this pattern.
#[clap(long, default_value = "\n\n")]
pub split_pattern: String,
/// Max. number of suggestions kept. If negative, all suggestions are kept.
#[clap(long, default_value_t = 5, allow_negative_numbers = true)]
pub max_suggestions: isize,
/// Specify the files type to use the correct parser.
///
/// If set to auto, the type is guessed from the filename extension.
#[clap(long, value_enum, default_value_t = FileType::default(), ignore_case = true)]
pub r#type: FileType,
/// Optional filenames from which input is read.
#[arg(conflicts_with_all(["text", "data"]), value_parser = parse_filename)]
pub filenames: Vec<PathBuf>,
/// Inner [`Request`].
#[command(flatten, next_help_heading = "Request options")]
pub request: Request,
}

#[cfg(test)]
mod request_tests {

use super::Request;

#[test]
fn test_with_text() {
let req = Request::default().with_text("hello".to_string());
let req = Request::default().with_text(std::borrow::Cow::Borrowed("hello"));

assert_eq!(req.text.unwrap(), "hello".to_string());
assert!(req.data.is_none());
}

#[test]
fn test_with_data() {
let req = Request::default().with_text("hello".to_string());
let req = Request::default().with_text(std::borrow::Cow::Borrowed("hello"));

assert_eq!(req.text.unwrap(), "hello".to_string());
assert!(req.data.is_none());
}
}

/// Responses
// RESPONSES

/// Detected language from check request.
#[allow(clippy::derive_partial_eq_without_eq)]
Expand Down Expand Up @@ -1027,17 +958,24 @@ impl Response {
#[derive(Debug, Clone, PartialEq)]
pub struct ResponseWithContext {
/// Original text that was checked by LT.
pub text: String,
pub text: Cow<'static, str>,
/// Check response.
pub response: Response,
/// Text's length.
pub text_length: usize,
}

impl Deref for ResponseWithContext {
type Target = Response;
fn deref(&self) -> &Self::Target {
&self.response
}
}

impl ResponseWithContext {
/// Bind a check response with its original text.
#[must_use]
pub fn new(text: String, response: Response) -> Self {
pub fn new(text: Cow<'static, str>, response: Response) -> Self {
let text_length = text.chars().count();
Self {
text,
Expand Down Expand Up @@ -1090,8 +1028,12 @@ impl ResponseWithContext {
}

self.response.matches.append(&mut other.response.matches);
self.text.push_str(other.text.as_str());

let mut string = self.text.into_owned();
string.push_str(other.text.as_ref());
self.text = Cow::Owned(string);
self.text_length += other.text_length;

self
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub mod languages;
pub mod server;
pub mod words;

use crate::error::{Error, Result};
use crate::error::Result;

/// A HTTP client for making requests to a LanguageTool server.
#[derive(Debug)]
Expand Down
Loading
Loading