vector-of-bool / pitchfork

Pitchfork is a Set of C++ Project Conventions
MIT License
1.08k stars 56 forks source link

New subcommand: update source list from source tree #7

Closed Quincunx271 closed 5 years ago

Quincunx271 commented 5 years ago

Adds a subcommand which updates a project's src/CMakeLists.txt from the source tree. Only supports CMake.

This inserts source files from the tree by scan the CMakeLists.txt file for function calls containing # sources comment. From that comment to the end of the function, everything is replaced with the globbed sources.

I don't intend on this being a permanent solution. My vision is to parse the CMakeLists.txt file and use some heuristics to determine where to place the source list.

This is very limited. It only globs for source files and header files; there's currently no option to glob for only one kind. It does skip sources directly under src/; it only grabs sources under a subdirectory of src/. It also only searches src/; it doesn't look at include/ or tests/

The git history for this branch is kind of messy. The commits are not that clean; there may be some commits which don't build, and there are some commits which are incomplete progress

Quincunx271 commented 5 years ago

Some bugs:

Edit: (I see now that a review would've been better) resolved

Quincunx271 commented 5 years ago

Another bug: pf update --help documents the --build-system flag as "The build system to generate". There's no build system being generated. Perhaps the build system flag should be a common flag between all commands, with a better description

Edit: resolved

Quincunx271 commented 5 years ago

Since I replaced std::experimental::filesystem with boost::filesystem, I should replace compare_fs.cpp L28 with fs::relative

Edit: resolved

Quincunx271 commented 5 years ago

Related to this PR: I have a project on my fork for what I'm planning in relation to this piece: https://github.com/Quincunx271/pitchfork/projects/1

Quincunx271 commented 5 years ago

Iteration order on std::filesystem's directory_iterator and recursive_directory_iterator is unspecified; the source list should be sorted for consistent ordering.

Edit: resolved

Quincunx271 commented 5 years ago

For the overwriting of the src/CMakeLists.txt, would it be better to instead write src/CMakeLists.txt.deleteme, then move that file over the old src/CMakeLists.txt? In other words, by writing directly over the old src/CMakeList.txt, am I at risk of corrupting it? Is that something we want to worry about?

vector-of-bool commented 5 years ago

Sorry to take so long on this. Are you feeling ready to merge this?

Quincunx271 commented 5 years ago

I'm feeling ready to merge this. There are more features I'd like in this, but those are probably better suited as separate PRs.

I'm thinking about whether the existing/ directory should be renamed to update/, though...