Make paths absolute when using compile_commands.json file #199
Conversation
Signed-off-by: caozhong <zhong.z.cao@intel.com>
jeaye
left a comment
jeaye
left a comment
There was a problem hiding this comment.
Thanks for the PR! I've some stylistic requests, but I think the change makes sense.
include/conf/load.hpp
Outdated
| return line; | ||
| } | ||
|
|
||
| void make_absolute(std::string &line, std::string &next, const fs::path &base) { |
There was a problem hiding this comment.
Since make_absolute makes a lot of assumptions about the data it's working with, with both line and next and their relationship, I think it'd be a cleaner approach to just have make_absolute work with the whole range. i.e.
template <typename It>
void make_absolute(It const &begin, It const &end, fs::path const &base)There was a problem hiding this comment.
I'm not sure I followed, make_absolute line/next is for the case where:
-I | -isystem | ...
../some/path/to/include
You mean there would be a case where:
-I | -isystem | ...
<empty line>
<empty line>
../some/path/to/include
Or just a single API make all directories absolute.
| line = make_absolute(line, base); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Will you please update the braces and const positioning to match the rest of the file? We use Allman-like braces, with a one-line exception, and const always goes to the right. You can find examples of each of these within this file.
…on file Signed-off-by: caozhong <zhong.z.cao@intel.com>
2 participants
As title. Also add a searching directory for lua library. @jeaye