Skip to content

Conversation

SimonCahill
Copy link

@SimonCahill SimonCahill commented Aug 15, 2025

Added

  • Adds isWhitespaceOrEmpty template function.
  • Adds support for concepts if enabled and supported by the compiler.

Changed

  • Replaces String::trim function with generic trim template functions which work with modern C++ types, such as string_view.

Removed

  • Removes String::trim function, in favor of generic trim template functions.

Fixed

  • Fixes broken logic in get_line function.
  • Fixes broken const-correctness in dbc.hpp and dbc.cpp.
  • Fixes logic error in unit test.
  • Fixes weird logic in get_next_non_blank_line and skip_to_next_blank_line by replacing an expensive regex with simpler and faster char checks.

@SimonCahill SimonCahill marked this pull request as ready for review August 15, 2025 14:30
@SimonCahill
Copy link
Author

I'd like to note here, that Mac OS isn't a platform anyone is using C++ 11 or higher on.
Mac OS X, macOS and their iOS counterparts use the standard Unix convention, of ending lines with \n; as they are based on BSD.

std::getline removes the trailing \n, so that extra check was removed.
I also took the liberty of removing expensive regex checks for blank lines, replacing them with simpler and quicker char checks. See: isspace

The file extension is not a definitive answer to anything. Shoving a JPEG image with a modified extension would falsely identify that file as a valid DBC.
@SimonCahill
Copy link
Author

I've also added better DBC detection; instead of relying on the file extension, which is in no way definitive, the checker now attempts to parse the header. If that fails, then we know the file is invalid.

@SimonCahill
Copy link
Author

Fixes #32

@SimonCahill SimonCahill changed the title Const Correctness Fixes Const Correctness and Bug Fixes Aug 15, 2025
@LinuxDevon
Copy link
Owner

LinuxDevon commented Sep 11, 2025

@SimonCahill Thank you for the PR and improvements! Sorry for the extreme delay. I missed the notification and haven't checked GH in awhile 🙃 Just have a few comments and questions, then I am good to merge

I've also added better DBC detection; instead of relying on the file extension, which is in no way definitive, the checker now attempts to parse the header. If that fails, then we know the file is invalid.

That was on my list of things to improve and thank you for taking care of that!

Also thank you for the regex speed improvements as well on the blank lines

@LinuxDevon
Copy link
Owner

LinuxDevon commented Sep 11, 2025

Also I just noticed none of the CI kicked off so there might be a bit more to handle. Looking into why that is right now.

EDIT: I just pushed some trigger changes. It should trigger the CI after the changes you push. You might have to rebase on top of main though

@SimonCahill
Copy link
Author

Hi @LinuxDevon!

No worries :) My customer ended up changing the requirements entirely so I no longer need the parser, but it was still fun to work on 😁👍

I'll get to answering any comments and questions, after I've had a chance to review exactly what I'd done.

Co-authored-by: Devon Adair <devonadair96@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants