Add "--list-languages" command line argument #79

connorkuehl · 2018-05-07T19:21:28Z

Closes #69

$ ./target/debug/bat.exe Cargo.toml --list-languages
ASP                              | asa
HTML (ASP)                       | asp
ActionScript                     | as
AppleScript                      | applescript, script editor
Batch File                       | bat, cmd
NAnt Build File                  | build
C#                               | cs, csx
C++                              | cpp, cc, cp, cxx, c++, C, h, hh, hpp, hxx, h++, inl, ipp
C                                | c, h
--snipped--

connorkuehl · 2018-05-07T19:23:58Z

src/main.rs

+
+                for lang in languages {
+                    print!("{:width$} | ", lang.name, width = longest);
+                    for i in 0..lang.file_extensions.len() {


Is there a more idiomatic way to list elements separated by a comma (except the last) than an indexed for-loop? I guess after using range loops and not having to deal with indices for so long I feel quite spoiled.

I think you should be able to use lang.file_extensions.join(", ").

That worked. Thank you!

sharkdp

Looks great, thank you very much work working on this!

sharkdp · 2018-05-07T19:29:51Z

src/main.rs

+        .arg(
+            Arg::with_name("list languages")
+                .long("list-languages")
+                .takes_value(false)


I think false is the default value, so .takes_value can be removed here.

sharkdp · 2018-05-07T19:31:31Z

src/main.rs

@@ -437,6 +437,12 @@ fn run() -> Result<()> {
                .default_value("auto")
                .help("When to use the pager"),
        )
+        .arg(
+            Arg::with_name("list languages")


I'd prefer if we use list-languages here, just like the .long(..) name.

sharkdp · 2018-05-07T19:31:35Z

src/main.rs

@@ -486,6 +492,31 @@ fn run() -> Result<()> {
                )
            })?;

+            if let Some(_) = app_matches.values_of("list languages") {


You can use if app_matches.is_present("list-languages").

sharkdp · 2018-05-07T19:32:19Z

src/main.rs

+
+                let longest = match languages.iter()
+                    .map(|s| s.name.len())
+                    .max() {


you can use languages.iter().....max().unwrap_or(32) here instead of the match statement.

sharkdp · 2018-05-07T19:37:50Z

src/main.rs

+                };
+
+                for lang in languages {
+                    print!("{:width$} | ", lang.name, width = longest);


Nice! Didn't know this was possible.

sharkdp · 2018-05-07T19:39:01Z

src/main.rs

+
+                for lang in languages {
+                    print!("{:width$} | ", lang.name, width = longest);
+                    for i in 0..lang.file_extensions.len() {


I think you should be able to use lang.file_extensions.join(", ").

sharkdp · 2018-05-07T19:48:32Z

Maybe we should also restrict the length of the file extensions?

Ruby                             | rb, Appfile, Appraisals, Berksfile, Brewfile, capfile, cgi, Cheffile, config.ru, Deliverfile, Fastfile, fcgi, Gemfile, gemspec, Guardfile, irbrc, jbuilder, podspec, prawn, rabl, rake, Rakefile, Rantfile, rbx, rjs, ruby.rail, Scanfile, simplecov, Snapfile, thor, Thorfile, Vagrantfile

😮

connorkuehl · 2018-05-07T21:49:49Z

Yes, we should limit the size of the extensions column. I had no idea some languages could have so many extensions lol.

I implemented simple line-wrapping for it. I'm still trying to learn more Rust, so if it is unnecessarily complex I would love some feedback to trim it down. I tried brainstorming a cool and simple iterator solution to this problem but didn't get too far.

Here's what it looks like:

--snipped--
reStructuredText                 | rst, rest
Ruby                             | rb, Appfile, Appraisals, Berksfile, Brewfile, capfile, cgi,
                                 | Cheffile, config.ru, Deliverfile, Fastfile, fcgi, Gemfile,
                                 | gemspec, Guardfile, irbrc, jbuilder, podspec, prawn, rabl,
                                 | rake, Rakefile, Rantfile, rbx, rjs, ruby.rail, Scanfile,
                                 | simplecov, Snapfile, thor, Thorfile, Vagrantfile
Cargo Build Results              |
Rust                             | rs
SQL                              | sql, ddl, dml
--snipped--

sharkdp · 2018-05-08T06:04:19Z

src/main.rs

+                    print!("{:width$}{}", lang.name, separator, width = longest);
+
+                    // Line-wrapping for the possible file extension overflow.
+                    let desired_width = 48;


Maybe this could be desired_width = term_width - longest - padding?

How should I get the term_width? I looked around and I'm not seeing a function I can call to get that value.

I notice it's set with a literal 90 in run(); should I just create a constant and apply it to both places?

The term_width is computed/retrieved in printer.rs. We should probably move that to the run function and store it in the Options struct.

sharkdp · 2018-05-08T06:21:10Z

That looks great, thank you very much!

Could you please try to resolve the merge conflicts? If you need help with that, let me know.

I implemented simple line-wrapping for it. I'm still trying to learn more Rust, so if it is unnecessarily complex I would love some feedback to trim it down. I tried brainstorming a cool and simple iterator solution to this problem but didn't get too far.

Your iterative version is completely fine, however... 😄

If you want to use this as an exercise to write iterator-based code, you would probably have to look at fold, because you need some way to store the state (e.g. num_chars) while iterating.

One option to do this could be to fold the list of extensions (["aa", "bb", .. "zz"]) into a list of lists ([["aa", "bb", "cc"], ["dd", "ee", "ff"], ...]). You could then call .join(", ") on the inner lists (via map) and call .join("\n ") (with the appropriate number of spaces) on the outer list.

I'm pretty sure the functional/iterator-based version will not be completely superior in terms of elegance, but it's worth a try (and a good exercise) 😄

sharkdp · 2018-05-08T18:33:18Z

We should probably also filter out Languages without any extensions. These are typically syntax definitions which are included by others, e.g. "Regular Expressions (Python)"

sharkdp · 2018-05-08T18:34:36Z

Actually, we should probably just hide the ones that have hidden == true (https://docs.rs/syntect/2.0.1/syntect/parsing/syntax_definition/struct.SyntaxDefinition.html)

sharkdp · 2018-05-08T19:07:33Z

src/main.rs

+                    print!("{:width$}{}", lang.name, separator, width = longest);
+
+                    // Line-wrapping for the possible file extension overflow.
+                    let desired_width = options.term_width - longest;


Don't we have to subtract separator.length() as well?

BrainMaestro · 2018-05-08T19:08:22Z

src/printer.rs

@@ -18,8 +17,7 @@ pub struct Printer<'a> {

 impl<'a> Printer<'a> {
    pub fn new(handle: &'a mut Write, options: &'a Options) -> Self {
-        let (_, term_width) = Term::stdout().size();
-        let term_width = term_width as usize;
+        let term_width = options.term_width;


Just replace all the usages of term_width with options.term_width then

sharkdp · 2018-05-08T19:08:31Z

src/main.rs

+                    let mut extension = lang.file_extensions.iter().peekable();
+                    while let Some(word) = extension.next() {
+                        // If we can't fit this word in, then create a line break and align it in.
+                        if word.len() + num_chars >= desired_width {


Do we have to check for word.len() + 2 + num_chars >= .. here due to the ", " part which is possibly appended?

sharkdp · 2018-05-08T19:44:18Z

src/printer.rs

@@ -30,7 +26,7 @@ impl<'a> Printer<'a> {
        Printer {
            handle,
            colors,
-            term_width,
+            term_width: options.term_width,


I think term_width can be removed from Printer if it is already included in options.

sharkdp

Thank you very much!

connorkuehl · 2018-05-08T19:55:06Z

Thanks for putting up with me! I appreciate it, I learned a lot.

sharkdp · 2018-05-08T19:59:59Z

src/main.rs

+                            print!("\n{:width$}{}", "", separator, width = longest);
+                        }
+
+                        num_chars += word.len();


Actually, this also needs to be num_chars += word.len() + comma_separator.len();. But I can also change this, if you want.

* removes redundant `.takes_value(false)`. * changes Arg name to "list-languages" to be consistent with long-form. * replaces unnecessary match statement with is_present(). * replaces unnecessary match statement on iter and uses unwrap_or() instead. * replaces for-loop with a call to join().

* Adds separator.length() to calculation for desired width. * Replaces use of term_width with options.term_width. * Adds the comma and space separator to calculation for line-wrapping.

connorkuehl commented May 7, 2018

View reviewed changes

sharkdp requested changes May 7, 2018

View reviewed changes

sharkdp reviewed May 8, 2018

View reviewed changes

sharkdp added this to the 0.3 milestone May 8, 2018

sharkdp reviewed May 8, 2018

View reviewed changes

BrainMaestro reviewed May 8, 2018

View reviewed changes

sharkdp reviewed May 8, 2018

View reviewed changes

sharkdp approved these changes May 8, 2018

View reviewed changes

sharkdp reviewed May 8, 2018

View reviewed changes

connorkuehl and others added 14 commits May 8, 2018 22:06

Add --list-languages argument to clap.

94b052f

Display language and extensions from loaded assets.

d4903a6

Size language name column width to the length of the longest language.

ff7672b

Rename and add comment to the match block.

13c81b9

Add simple line-wrapping for file extensions.

1de832d

WIP edit desired width.

71f2e27

Filter out languages that are hidden.

6e5b00b

Add term_width to options struct, move getting term_width to run()

3ca4dd1

Add revisions for line-wrapping and terminal width.

b5bd269

* Adds separator.length() to calculation for desired width. * Replaces use of term_width with options.term_width. * Adds the comma and space separator to calculation for line-wrapping.

Remove term_width from Printer, its options owns it.

7da80c9

Correctly update num_chars

352c29e

Compute 'longest' for non-hidden languages only

dc0ac85

Paint file extensions in green

956811d

Filter out languages without extensions

52cff1f

sharkdp merged commit f7af537 into sharkdp:master May 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "--list-languages" command line argument #79

Add "--list-languages" command line argument #79

connorkuehl commented May 7, 2018

connorkuehl May 7, 2018

sharkdp May 7, 2018

connorkuehl May 7, 2018

sharkdp left a comment

sharkdp May 7, 2018

sharkdp May 7, 2018

sharkdp May 7, 2018

sharkdp May 7, 2018

sharkdp May 7, 2018

sharkdp May 7, 2018

sharkdp commented May 7, 2018

connorkuehl commented May 7, 2018

sharkdp May 8, 2018

connorkuehl May 8, 2018

sharkdp May 8, 2018 •

edited

Loading

sharkdp commented May 8, 2018

sharkdp commented May 8, 2018

sharkdp commented May 8, 2018

sharkdp May 8, 2018

BrainMaestro May 8, 2018

sharkdp May 8, 2018

sharkdp May 8, 2018

sharkdp left a comment

connorkuehl commented May 8, 2018

sharkdp May 8, 2018

Add "--list-languages" command line argument #79

Add "--list-languages" command line argument #79

Conversation

connorkuehl commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp commented May 7, 2018

connorkuehl commented May 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp May 8, 2018 • edited Loading

Choose a reason for hiding this comment

sharkdp commented May 8, 2018

sharkdp commented May 8, 2018

sharkdp commented May 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkdp left a comment

Choose a reason for hiding this comment

connorkuehl commented May 8, 2018

Choose a reason for hiding this comment

sharkdp May 8, 2018 •

edited

Loading