search

Sarcasm / irony-mode

A C/C++ minor mode for Emacs powered by libclang
GNU General Public License v3.0
901 stars 98 forks source link

Support for compile_flags.txt #556

Closed vaivaswatha closed 4 years ago

vaivaswatha commented 4 years ago

This addresses the comment in #505, which looks to be abandoned.

Closes #489.

Sarcasm commented 4 years ago

Thanks! It's slightly better than the previous one, but there is still 2 traversals if the user is using compile_flags.txt. I'm pretty one call to locate-dominating-file is sufficient, can you try that?

vaivaswatha commented 4 years ago

Thanks! It's slightly better than the previous one, but there is still 2 traversals if the user is using compile_flags.txt. I'm pretty one call to locate-dominating-file is sufficient, can you try that?

Done.

Sarcasm commented 4 years ago

Just a thought, as I feel like this could simplify the code even further (but could be wrong).

Can you throw from the locate-dominating-file function, and return the value if you catch any? Also, I would favor looking for .compile_flags.txt first, as it is now a somewhat standard file, documented by Clang itself.

If you don't think it's worth it, I'm ok merging the code as-is.

vaivaswatha commented 4 years ago

Just a thought, as I feel like this could simplify the code even further (but could be wrong).

Can you throw from the locate-dominating-file function, and return the value if you catch any? Also, I would favor looking for .compile_flags.txt first, as it is now a somewhat standard file, documented by Clang itself.

If you don't think it's worth it, I'm ok merging the code as-is.

Pushed both the changes you suggested.

Sarcasm commented 4 years ago

Great! I think the last thing that could have been improved is the double-ifs. Using a when, cond or maybe pcase could make this part slightly more succinct.

Thank you for your time and effort. :+1: