mity / md4c

C Markdown parser. Fast. SAX-like interface. Compliant to CommonMark specification.
MIT License
756 stars 138 forks source link

Strengthen build #132

Closed rsms closed 3 years ago

rsms commented 3 years ago

This set of changes improves some minor things, allowing compilation with stronger warnings (-Wextra) to reduce risk of undefined behavior. This patch also removes a few unused parameters which should yield smaller and maybe faster generated code. It also fixes an uninitialized variable (in md_analyze_emph) and partially initialized struct (md_dummy_blank_line).

In addition this contains a build script I find useful myself, which I've used in other projects. It's added to the scripts directory and is completely optional.

codecov[bot] commented 3 years ago

Codecov Report

Merging #132 into master will decrease coverage by 0.52%. The diff coverage is 94.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   94.38%   93.85%   -0.53%     
==========================================
  Files           3        3              
  Lines        3061     3061              
==========================================
- Hits         2889     2873      -16     
- Misses        172      188      +16     
Impacted Files Coverage Δ
src/md4c.c 93.72% <94.73%> (-0.58%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 002f76c...af66338. Read the comment docs.

mity commented 3 years ago

Hello,

sorry for the late reply. I went finally over the patches, I am fine with some, not so with others:

  • 0812df7 - adds -watch option to scripts/build.sh for rebuilding as source files change
  • f4f8994 - adds -clean option to scripts/build.sh and places debug & release builds in separate subdirectories under build
  • 4e75722 - adds -test flag to scripts/build.sh to run tests after successful build
  • ec313b4 - adds CLI options to scripts/build.sh and strenghten compiler diagnostics
  • 8707b85 - adds scripts/build.sh for building the project

I use a similar script located in my $PATH and many other people do as well, so I am not that much in a favor in a project a script I never use/test/maintain at all.

  • af66338 - md2html: decorates data arg in cmdline_callback as unused
  • b78d49c - md4c: removes or declares unused function arguments

I can imagine marking the unused arguments, if it silences some compiler warnings. However, better to do it in a compiler-agnostic way:

void func(int unused_arg)
{
    (void) unused_arg;

    ...
}

So not merged.

  • 59efc10 - Adds missing field initializers (undefined behavior)

Merged.

  • f9581a9 - Fixes sign mismatch comparison

This change causes makes the test to report a regression:

Code coverage & regressions:
Example 17 (lines 186-191) [Issue 78](https://github.com/mity/md4c/issues/78)
[SS ẞ]: /url
[ẞ SS]

So not merged.

  • 31c91e2 - Initializes an uninitilized variable in md_analyze_emph

Merged, why not. Even though for any uninitialized variable there is always at least one compiler or its version or its combination of flags which warns about it being possibly used uninitialized...

  • a338a14 - adds /build dir to gitignore

Merged.

mity commented 3 years ago

(Some commits cherry-picked inidividually. Please consider next time to create individual PRs for unrelated stuff.)