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 percent column #258

Closed
Closed
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
9 changes: 9 additions & 0 deletions src/language/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,12 @@ impl AddAssign for Language {
}
}

impl<'a> AddAssign<&'a Self> for Language {
fn add_assign(&mut self, rhs: &Self) {
self.lines += rhs.lines;
self.comments += rhs.comments;
self.blanks += rhs.blanks;
self.code += rhs.code;
self.stats.extend(rhs.stats.clone());
}
}
88 changes: 67 additions & 21 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ fn main() -> Result<(), Box<Error>> {
or \"stdin\" to read from stdin.")
(@arg files: -f --files
"Will print out statistics on individual files.")
(@arg percent: -p --percent
"Will print out regular statistics with percentages related to the rest of the project.")
(@arg input:
conflicts_with[languages] ...
"The input file(s)/directory(ies) to be counted.")
Expand Down Expand Up @@ -149,6 +151,7 @@ fn main() -> Result<(), Box<Error>> {
}
ignored_directories
};
let percent_option = matches.is_present("percent");

// Sorting category should be restricted by clap but parse before we do work just in case.
let sort_category = sort_option.map(parse_or_exit::<Sort>);
Expand Down Expand Up @@ -185,6 +188,11 @@ fn main() -> Result<(), Box<Error>> {
});

languages.get_statistics(&paths, ignored_directories, types);
let mut total = Language::new();

for (_, language) in &languages {
total += language;
}

if let Some(format) = output_format {
print!("{}", format.print(languages).unwrap());
Expand Down Expand Up @@ -226,37 +234,31 @@ fn main() -> Result<(), Box<Error>> {
Sort::Lines => languages.sort_by(|a, b| b.1.lines.cmp(&a.1.lines)),
}

print_results(&mut stdout, &row, languages.into_iter(), files_option)?
print_results(&mut stdout, &row, languages.into_iter(), &total, files_option, percent_option)?
} else {
print_results(&mut stdout, &row, languages.iter(), files_option)?
print_results(&mut stdout, &row, languages.iter(), &total, files_option, percent_option)?
}

// If we're listing files there's already a trailing row so we don't want an extra one.
if !files_option {
writeln!(stdout, "{}", row)?;
}

let mut total = Language::new();

for (_, language) in languages {
total += language;
}

print_language(&mut stdout, columns - NO_LANG_ROW_LEN, &total, "Total")?;
print_language(&mut stdout, columns - NO_LANG_ROW_LEN, &total, &total, "Total", percent_option)?;
writeln!(stdout, "{}", row)?;

Ok(())
}

fn print_results<'a, I, W>(sink: &mut W, row: &str, languages: I, list_files: bool)
fn print_results<'a, I, W>(sink: &mut W, row: &str, languages: I, total: &Language, list_files: bool, show_percentage: bool)
-> io::Result<()>
where I: Iterator<Item = (&'a LanguageType, &'a Language)>,
W: Write,
{
let path_len = row.len() - NO_LANG_ROW_LEN_NO_SPACES;
let lang_section_len = row.len() - NO_LANG_ROW_LEN;
for (name, language) in languages.filter(isnt_empty) {
print_language(sink, lang_section_len, language, name.name())?;
print_language(sink, lang_section_len, language, total, name.name(), show_percentage)?;

if list_files {
writeln!(sink, "{}", row)?;
Expand All @@ -277,17 +279,61 @@ fn isnt_empty(&(_, language): &(&LanguageType, &Language)) -> bool {
fn print_language<W>(sink: &mut W,
lang_section_len: usize,
language: &Language,
name: &str)
total: &Language,
name: &str,
show_percentage: bool)
-> io::Result<()>
where W: Write,
{
writeln!(sink,
" {:<len$} {:>6} {:>12} {:>12} {:>12} {:>12}",
name,
language.stats.len(),
language.lines,
language.code,
language.comments,
language.blanks,
len = lang_section_len)
if !show_percentage {
return writeln!(
sink,
" {:<len$} {:>6} {:>12} {:>12} {:>12} {:>12}",
name,
language.stats.len(),
language.lines,
language.code,
language.comments,
language.blanks,
len = lang_section_len)
}

let percent_char_count = 5;
// don't show percentage for the total row because that would be pointless
if language.lines == total.lines {
Copy link
Owner

@XAMPPRocky XAMPPRocky Sep 17, 2018

Choose a reason for hiding this comment

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

Instead of passing in total and performing this if statement, why not call total's print_language with show_percentage as always false.

Copy link
Author

Choose a reason for hiding this comment

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

I still need to perform special formatting for total when show_percentage is true. It's so that the total number of files line up well. You can see the difference in the last two outputs I posted

Copy link
Owner

Choose a reason for hiding this comment

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

I think then that there should probably be a seperate function for printing total. Currently this implementation changes a lot of behaviour (such as moving the total around which adds additional allocations in AddAssign) which is not desired.

Copy link
Author

Choose a reason for hiding this comment

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

How would you suggest I do that?

Copy link
Owner

Choose a reason for hiding this comment

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

So I've refactored all the printing code into src/cli_utils.rs so you'll need to move your changes to that. I would suggest having a function called print_total that handles all the logic of printing out the total.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not able to rework this now, so I'll pick it up again later when I can. I'll just go ahead and close the PR until then. thanks

Copy link
Owner

@XAMPPRocky XAMPPRocky Sep 21, 2018

Choose a reason for hiding this comment

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

@BrainMaestro Okay, thank you again for your PR! I'm going to take some time and write a comment on the issue detailing exact requirements so that if and when you do get back to it you'll have a much clearer sense of the task.

Copy link
Author

Choose a reason for hiding this comment

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

@Aaronepower that sounds great.

Copy link

Choose a reason for hiding this comment

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

I'm going to take some time and write a comment on the issue detailing exact requirements

Hi, was this ever written? I'm interested in trying to (re)implement this. Although I'm new to Rust and open source.

// start the total number of fule just before the percentages, in line with the bar
writeln!(sink,
" {:<len$} {:>7}{:<4} {:>12} {:>12} {:>12} {:>12}",
name,
language.stats.len(),
"",
language.lines,
language.code,
language.comments,
language.blanks,
len = lang_section_len - percent_char_count)
} else {
let percentage = get_percentage(language.lines, total.lines);
writeln!(sink,
" {:<len$} {:>5}|{:<5} {:>12} {:>12} {:>12} {:>12}",
name,
language.stats.len(),
percentage,
language.lines,
language.code,
language.comments,
language.blanks,
len = lang_section_len - percent_char_count)
}
}

fn get_percentage(lines: usize, total_lines: usize) -> String {
let percentage = lines as f64 * 100_f64 / total_lines as f64;
if percentage > 10_f64 {
format!("{}%", percentage as usize)
} else if percentage > 1_f64 {
format!("{:.1}%", percentage)
} else {
format!("{:.2}%", percentage)
}
}