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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Markdown is ignored if it doesn't contain code blocks #933

Closed
1 task done
spenserblack opened this issue Jan 17, 2023 · 4 comments 路 Fixed by #937
Closed
1 task done

Markdown is ignored if it doesn't contain code blocks #933

spenserblack opened this issue Jan 17, 2023 · 4 comments 路 Fixed by #937
Assignees
Labels
bug Something isn't working

Comments

@spenserblack
Copy link
Collaborator

Duplicates

  • I have searched the existing issues

Current behavior 馃槸

Running onefetch -T prose on a repo that contains Markdown results in

Error: Could not find any source code in this repository

Expected behavior 馃

Markdown should be detected if prose is enabled, even if the Markdown doesn't contain code blocks.

Steps to reproduce 馃暪

TMPDIR=$(mktemp -d)
cd $TMPDIR
git init
git commit --allow-empty -m "init"  # just adding a commit for some stats
cat <<EOF > file.md
# hello, world

This is a file without code blocks
EOF
onefetch -T prose  # error

cat << "EOF" > file.md
```bash
chmod +x install.sh
./install.sh
```
EOF
onefetch -T prose  # renders Markdown ASCII

Additional context/Screenshots 馃敠

This looks similar to #627, but -T prose alone doesn't resolve this.

Possible Solution 馃挕

The issue seems to be that tokei marks lines of Markdown as comments (then what is <!-- -->? 馃槅) except for code blocks and blank lines. IMO all lines of Markdown should be counted (even blank lines, since they can have significance in the rendered output).

Unfortunately, this is a bit hard to do since all languages are defined in a data file. Perhaps an optional field called something like tokei-fields can be used, which would allow one to list what types of lines should be used ([Code, Comments, Blanks]).

@spenserblack spenserblack added the bug Something isn't working label Jan 17, 2023
@spenserblack
Copy link
Collaborator Author

spenserblack commented Jan 17, 2023

We could change this

let mut code = language.code;

into something like

let code = language::compile_loc(language)

And the terra code can loop through tokei-fields || [code], and do lines += language.{{ field }}.

Or perhaps a CLI arg that allows what "type" of line is allowed?

We can always hard-code Markdown to be a special case, but that won't be pretty (but is there a pretty solution to this?) 馃槅

@o2sh
Copy link
Owner

o2sh commented Jan 18, 2023

We can always hard-code Markdown to be a special case

It's indeed very tempting 馃槄
To a lesser extent, we may also encounter a similar problem with jupyter notebooks.

The solution using tera with a CLI flag is great and would be a nice feature by itself, but it would impact the LOC for all languages detected inside the repo and not just Markdown.

@spenserblack
Copy link
Collaborator Author

I guess if we're giving the user control, it wouldn't just be "LOC types", but "LOC types" per language. Which is way to complex for CLI, and would probably belong in a config file. But that's if we give users controls. We probably want to keep some consistency in results.

The CLI arg suggestion was basically just me being lazy and moving the responsibility to the user 馃槅

So I'm thinking

Jupyter:
  # ...
  tokei-lang-fields: [code, comments]

Luckily I don't think we have too many languages to apply this too, so the biggest challenge is working with the tera template.

@o2sh
Copy link
Owner

o2sh commented Jan 18, 2023

Very nice solution. This means that only markdown and Jupyter - for now - would have this additional parameter leaving the other language entries unchanged (that would default to just [code]).

@spenserblack spenserblack self-assigned this Jan 19, 2023
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
spenserblack added a commit to spenserblack/onefetch that referenced this issue Jan 19, 2023
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`.

Fixes o2sh#933
@o2sh o2sh closed this as completed in #937 Jan 24, 2023
o2sh added a commit that referenced this issue Jan 24, 2023
* Conditionally count lines based on language type

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`.

Fixes #933

* Simplify name of LOC counting function

"compile" is a bit of a misnomer, since it's related to any compilation,
but merely counts the number of lines of code for a language.

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>

* Reduce clutter of output `loc` function

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>

* Use templated `loc` function for total LOC

* Remove reference to tokei from languages file

This references internal logic, and is unnecessarily wordy. Instead,
this field is now described in CONTRIBUTING to explain what it is and
what values it can take.

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>

* Add unit test for `get_total_loc`

* Recursively count code stats LOC

* Unset line types for Jupyter Notebooks

All lines are counted from the language's children.

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>

* Remove unnecessary recursion

`.summarise()` accomplishes what recursion was doing.

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>

* Refactor to make templated functions private

This adds public wrapper functions to the templated functions and makes
them private. This makes it so as much code is in actual Rust code, and
leaves the template to only manage the repetitive code.

* Add Jupyter total LOC test

Co-authored-by: Ossama Hjaji <ossama-hjaji@live.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants