Skip to content

Commit

Permalink
Add some performance optimizations (#18)
Browse files Browse the repository at this point in the history
  • Loading branch information
mondeja authored Feb 1, 2025
1 parent 22bfed7 commit ef2f27d
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 111 deletions.
37 changes: 27 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,56 @@ jobs:
qa:
name: QA
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
job: [pre-commit, lychee, cargo-machete]
steps:
- uses: actions/checkout@v4
- name: Setup Rust
if: matrix.job == 'cargo-machete'
uses: hecrj/setup-rust-action@v2
with:
rust-version: stable
profile: minimal
- name: Set up Python
if: matrix.job == 'pre-commit'
uses: actions/setup-python@v5
with:
python-version: "3.12"
- name: Install dependencies
if: matrix.job == 'pre-commit'
run: |
ls -la
ls -la ..
pip install --upgrade pip
pip install pre-commit
- name: Cache dependencies
uses: actions/cache@v4
with:
path: |
~/.cargo/registry
~/.cargo/git
target
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }}-${{ hashFiles('**/Cargo.lock') }}
- name: Lint
if: matrix.job == 'pre-commit'
uses: nick-fields/retry@v3
with:
timeout_minutes: 25
max_attempts: 3
retry_wait_seconds: 15
warning_on_retry: false
command: pre-commit run --all-files --show-diff-on-failure
- name: Restore lychee cache
if: matrix.job == 'lychee'
uses: actions/cache@v4
with:
path: .lycheecache
key: cache-lychee-${{ github.sha }}
restore-keys: cache-lychee-
- name: Run Lychee
if: matrix.job == 'lychee'
uses: lycheeverse/lychee-action@v2
with:
args: "--cache --max-cache-age 1d ."
- uses: taiki-e/install-action@v2
if: matrix.job == 'cargo-machete'
with:
tool: cargo-machete
- name: Run cargo-machete
if: matrix.job == 'cargo-machete'
run: cargo machete --skip-target-dir

build:
name: Build hledger-fmt
Expand Down
10 changes: 0 additions & 10 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,3 @@ repos:
-D,
clippy::semicolon_if_nothing_returned,
]
- repo: https://github.com/mondeja/rust-pc-hooks
rev: v1.3.0
hooks:
- id: cargo-machete
args:
- --skip-target-dir
- repo: https://github.com/mondeja/rust-pc-hooks
rev: v1.3.0
hooks:
- id: lychee
40 changes: 20 additions & 20 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ use clap::{value_parser, Arg, ArgAction, Command};
pub fn cli() -> Command {
Command::new("hledger-fmt")
.long_about("An opinionated hledger's journal files formatter.")
.override_usage(concat!("hledger-fmt [OPTIONS] [FILES]...\n"))
.override_usage("hledger-fmt [OPTIONS] [FILES]...\n")
.arg(
Arg::new("files")
.help(concat!(
"Paths of files to format. To read from STDIN pass '-'.\n",
"\n",
"If not defined, hledger-fmt will search for hledger files in the",
" current directory and its subdirectories (those that have the",
" extensions '.journal', '.hledger' or '.j').",
" If the paths passed are directories, hledger-fmt will search for",
" hledger files in those directories and their subdirectories.",
))
.help(
"Paths of files to format. To read from STDIN pass '-'.\n\
\n\
If not defined, hledger-fmt will search for hledger files in the \
current directory and its subdirectories (those that have the \
extensions '.journal', '.hledger' or '.j'). \
If the paths passed are directories, hledger-fmt will search for \
hledger files in those directories and their subdirectories.",
)
.action(ArgAction::Append)
.value_parser(value_parser!(String))
.value_name("FILES")
Expand All @@ -23,21 +23,21 @@ pub fn cli() -> Command {
.arg(
Arg::new("fix")
.long("fix")
.help(concat!(
"Fix the files in place. WARNING: this is a potentially destructive",
" operation, make sure to make a backup of your files or print the diff",
" first. If not passed, hledger-fmt will print the diff between the",
" original and the formatted file."
))
.help(
"Fix the files in place. WARNING: this is a potentially destructive \
operation, make sure to make a backup of your files or print the diff \
first. If not passed, hledger-fmt will print the diff between the \
original and the formatted file.",
)
.action(ArgAction::SetTrue),
)
.arg(
Arg::new("no-diff")
.long("no-diff")
.help(concat!(
"Don't print diff between original and formatted files,",
" but formatted content instead."
))
.help(
"Don't print diff between original and formatted files, \
but formatted content instead.",
)
.action(ArgAction::SetTrue),
)
.disable_help_flag(true)
Expand Down
124 changes: 72 additions & 52 deletions src/formatter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,30 @@ use crate::{
TransactionNode,
},
};
use core::fmt::Write;

#[derive(Default)]
pub struct FormatContentOptions {
estimated_length: usize,
}

impl FormatContentOptions {
pub fn new() -> Self {
Self::default()
}

pub fn with_estimated_length(mut self, estimated_length: usize) -> Self {
self.estimated_length = estimated_length;
self
}
}

pub fn format_content(nodes: &JournalFile) -> String {
let mut formatted = String::new();
format_content_with_options(nodes, &FormatContentOptions::default())
}

pub fn format_content_with_options(nodes: &JournalFile, opts: &FormatContentOptions) -> String {
let mut formatted = String::with_capacity(opts.estimated_length);

for node in nodes {
match node {
Expand All @@ -20,18 +41,19 @@ pub fn format_content(nodes: &JournalFile) -> String {
colno,
..
}) => {
formatted.push_str(&format!(
"{}{}{}\n",
_ = writeln!(
formatted,
"{}{}{}",
" ".repeat(*colno - 1),
*prefix as u8 as char,
content
));
);
}
JournalCstNode::EmptyLine { .. } => {
formatted.push('\n');
}
JournalCstNode::MultilineComment { content, .. } => {
formatted.push_str(&format!("comment\n{content}end comment\n"));
_ = writeln!(formatted, "comment\n{content}end comment");
}
JournalCstNode::DirectivesGroup {
nodes,
Expand All @@ -46,38 +68,36 @@ pub fn format_content(nodes: &JournalFile) -> String {
comment,
..
}) => {
formatted.push_str(&format!(
"{} {}{}\n",
name,
content,
match comment {
Some(comment) => {
format!(
"{}{}{}",
" ".repeat(
2 + max_name_content_len
- name.chars().count()
- content.chars().count()
),
comment.prefix as u8 as char,
comment.content
)
}
None => String::new(),
},
));
_ = write!(formatted, "{name} {content}");

if let Some(comment) = comment {
_ = write!(
formatted,
"{}{}{}",
" ".repeat(
2 + max_name_content_len
- name.chars().count()
- content.chars().count()
),
comment.prefix as u8 as char,
comment.content
);
}

formatted.push('\n');
}
DirectiveNode::SingleLineComment(SingleLineComment {
content,
prefix,
..
}) => {
formatted.push_str(&format!(
"{}{}{}\n",
_ = writeln!(
formatted,
"{}{}{}",
" ".repeat(*max_name_content_len + 3),
*prefix as u8 as char,
content
));
);
}
}
}
Expand All @@ -99,16 +119,15 @@ pub fn format_content(nodes: &JournalFile) -> String {
max_entry_value_third_part_decimal_len,
max_entry_value_third_part_numeric_units_len,
} => {
formatted.push_str(&format!(
"{}{}\n",
title.trim(),
match title_comment {
Some(comment) => {
format!(" {}{}", comment.prefix as u8 as char, comment.content)
}
None => String::new(),
}
));
_ = write!(formatted, "{}", title.trim());
if let Some(comment) = title_comment {
_ = write!(
formatted,
" {}{}",
comment.prefix as u8 as char, comment.content
);
}
formatted.push('\n');

for entry in entries {
match entry {
Expand Down Expand Up @@ -222,7 +241,9 @@ pub fn format_content(nodes: &JournalFile) -> String {
value_third_part_decimal,
);

let comment_part = if let Some(comment) = comment {
formatted.push_str(&entry_line);

if let Some(comment) = comment {
let comment_separation = if !value_second_separator.is_empty() {
2 + max_entry_value_third_part_decimal_len
- value_third_part_decimal.chars().count()
Expand All @@ -236,39 +257,38 @@ pub fn format_content(nodes: &JournalFile) -> String {
- value_first_part_decimal.chars().count()
};

format!(
let title_chars_count = title.chars().count();
let entry_line_chars_count = entry_line.chars().count();

_ = write!(
formatted,
"{}{}{}",
" ".repeat(
if title.chars().count() + 2
> entry_line.chars().count() + 2
{
title.chars().count() + 2 - entry_line.chars().count()
if title_chars_count + 2 > entry_line_chars_count + 2 {
title_chars_count + 2 - entry_line_chars_count
} else {
comment_separation
}
),
comment.prefix as u8 as char,
comment.content
)
} else {
String::new()
);
};

formatted.push_str(&entry_line);
formatted.push_str(&comment_part);
formatted.push('\n');
}
TransactionNode::SingleLineComment(SingleLineComment {
content,
prefix,
..
}) => {
formatted.push_str(&format!(
"{}{}{}\n",
_ = writeln!(
formatted,
"{}{}{}",
" ".repeat(*first_entry_indent),
*prefix as u8 as char,
content
));
);
}
}
}
Expand Down
15 changes: 7 additions & 8 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,9 @@ fn main() {
if files.is_empty() {
eprintln!(
"{}",
concat!(
"No hledger journal files found in the current directory nor its",
" subdirectories.\nEnsure that have extensions '.hledger', '.journal'",
" or '.j'."
)
// Rewrite without concatenation
"No hledger journal files found in the current directory nor its subdirectories.\n\
Ensure that have extensions '.hledger', '.journal' or '.j."
);
exitcode = 1;
std::process::exit(exitcode);
Expand Down Expand Up @@ -104,7 +102,9 @@ fn main() {
continue;
}
let parsed = parsed_or_err.unwrap();
let formatted = hledger_fmt::formatter::format_content(&parsed);
let format_opts = hledger_fmt::formatter::FormatContentOptions::new()
.with_estimated_length(content.len());
let formatted = hledger_fmt::formatter::format_content_with_options(&parsed, &format_opts);
if formatted == content {
continue;
}
Expand Down Expand Up @@ -231,8 +231,7 @@ fn read_file(file_path: &str) -> Result<String, ()> {

fn read_stdin() -> String {
let mut buffer = String::new();
let lines = std::io::stdin().lines();
for line in lines {
for line in std::io::stdin().lines() {
buffer.push_str(&line.unwrap());
buffer.push('\n');
}
Expand Down
Loading

0 comments on commit ef2f27d

Please sign in to comment.