Fix Markdown / Jupyter markup not getting counted

[spenserblack]

This allows an optional definition of the line type for a language. If it is defined, it will sum them together. Defaults to using .code. By using this definition, comments are included in the type of lines of code to be summed for Markdown and Jupyter Notebooks.

This also changes the summing of child languages to be recursive, so that deeply nested code (e.g. Bash in Markdown in Jupyter) can be counted.

Fixes #933

[spenserblack]

I can add a test or two if needed.

Edit: Added a couple of tests.

[o2sh]

Nicely done!

Left a few remarks.

Don't forget to apply the same logic to the get_total_loc function.

[spenserblack]

Thanks for the review! I'll probably get to get_total_loc over the weekend.

[o2sh]

language::loc should probably also be applied here in case one of the children language happens to be Markdown.

Jupyter notebooks is a good example:

[spenserblack]

That's a good point. I don't use Jupyter notebooks, so I thought Markdown was the "top level" code. Could that Markdown then contain its own code blocks? If so, perhaps this function should be recursive for deeply nested code blocks?

[o2sh]

tokei's parser is indeed recursive, as stated in its changelog:

[spenserblack]

Deeply nested stats are now recursively counted with 939e9e1

[spenserblack]

Oops, I thought that I would be labelled the committer and you would be the author of f41bc02. I guess codespaces behave differently when --author= is used. Hope you don't mind that I made a commit that looks like you.

[spenserblack]

As something to do in the future, I think it would be nice to collect code samples like linguist and tokei do, and perform some snapshot tests on the CLI output using those samples. As a lazy test, I used

{
 "cells": [
  {
   "attachments": {},
   "cell_type": "markdown",
   "metadata": {},
   "source": [
    "## Setup\n",
    "\n",
    "```bash\n",
    "echo 'Hello, World!'\n",
    "```"
   ]
  }
 ],
 "metadata": {
  "kernelspec": {
   "display_name": "Python 3",
   "language": "python",
   "name": "python3"
  },
  "language_info": {
   "codemirror_mode": {
    "name": "ipython",
    "version": 3
   },
   "file_extension": ".py",
   "mimetype": "text/x-python",
   "name": "python",
   "nbconvert_exporter": "python",
   "pygments_lexer": "ipython3",
   "version": "3.4.3"
  }
 },
 "nbformat": 4,
 "nbformat_minor": 0
}

And got 4 lines as expected (Jupyter contains Markdown, Markdown contains 3 comment lines and 1 line of Bash).

[spenserblack]

Just checked the source code of .summarise, and it looks like we'll lose some granularity.
Before aa31a72, a comment in a bash code block in Markdown in Jupyter would be ignored, since only code is counted for Bash. Summarise would combine the CodeStats from the Bash with the Markdown, making all comments get counted, as long as the 1st-level child is Markdown.

Which may be preferable behavior (it makes sense to me IMO), but I just want to throw that out there.

[o2sh]

Don't you think it would be better to keep the code logic outside of the tera template? I'm refering to lines 123-125 and 131.

We could probably extract those lines and put them back in get_language_distribution() between line 34 and 36:

let has_children = !language.children.is_empty();

if has_children {
    for (lang_type, reports) in language.children.iter() {
        loc += reports
            .iter()
            .map(|r| r.stats.summarise())
            .fold(0, |sum, stats| sum + language::stats_loc(lang_type, &stats));
    }
}

this would simplify the template but more importantly avoid obfuscating part of the code logic.

Leaving two methods loc and stats_loc inside the template which would be almost identical except for one of their parameters. I wonder if there is a way we could combine the two methods 🤔 (maybe in the future)

[spenserblack]

🤔 Yeah, it does obfuscate it a bit by hiding the logic in the template. But my thinking was that I'd like to avoid needing to repeat similar logic (adding the LOC of the children) for both language distributions and total line counts. Since these are kind of private methods, maybe they should be #[doc(hidden)] and called __loc(), and have more visible methods called loc() and stats_loc() in language.rs that call these methods and act as wrappers.

fn loc(language_type: &tokei::LanguageType, language: &tokei::Language) -> usize {
    __loc(language_type, language) + language
        .children 
        // ...

What do you think?

[o2sh]

As you mentioned in your comment #937 (comment)
We do need additional tests covering more complex cases (language with children f. ex.) to make sure we don't break things. Should not block this PR, but preferably before the next release.

[o2sh]

But my thinking was that I'd like to avoid needing to repeat similar logic (adding the LOC of the children) for both language distributions and total line counts.

You're right, I missed that 🤦

Since these are kind of private methods, maybe they should be #[doc(hidden)] and called __loc(), and have more visible methods called loc() and stats_loc() in language.rs that call these methods and act as wrappers.

I'm sorry, but I'm lost - too many loc 😅 - maybe you can commit your suggestion to illustrate your idea. But yeah, if we could avoid putting logic inside the tera template, that would be great. we could probably factorize this part inside a method:

let mut loc = language::loc(language_name, language);

let has_children = !language.children.is_empty();

if has_children {
    for (lang_type, reports) in language.children {
        loc += reports
            .iter()
            .map(|r| r.stats.summarise())
            .fold(0, |sum, stats| sum + language::stats_loc(&lang_type, &stats));
    }
}

and call it for both language distributions and total line counts.

But it's your call, do what you think is best 👍 I think this PR is already pretty good.

[spenserblack]

maybe you can commit your suggestion to illustrate your idea.

Sure! 64077e7 is the basic idea. __<function> contains the repetitive code that the template manages, and <function> is a wrapper that calls that function and includes some helper code.

[spenserblack]

I'll probably add a test for Jupyter code and call it on get_total_loc. That way loc and stats_loc is 100% covered. __loc's template would be 100% covered, but the generated code would probably be <10% with each language being a branch 😆
I think I'll just hardcode the structs necessary to simulate Jupyter code.

[o2sh]

Perfect 🎊 , thanks @spenserblack