cquery-project / emacs-cquery

Emacs client for cquery, a low-latency language server supporting multi-million line C++ code-bases
116 stars 14 forks source link

Default `cquery-enable-inactive-region' to nil and fix custom type #46

Closed wyuenho closed 6 years ago

wyuenho commented 6 years ago

Inactive region highlight doesn't seem to have a very good idea of what's actually active and cquery just decides to treat every macro as comment, this isn't very ideal, so I suggest you to disable it by default for now. I've also fixed the custom type in this PR.

screen shot 2018-06-23 at 22 20 12
jacobdufault commented 6 years ago

Likely your project configuration is not setup correctly; inactive region highlight is showing what is not being indexed by clang. If it is not, then that may be a bug.

wyuenho commented 6 years ago

The project config is generated by bear -a. That part is probably not indexed by clang as I was working on a Mac, but it still doesn't make sense to enable inactive region by default since cross-platform code is fairely common.

jacobdufault commented 6 years ago

Cross platform code is where you want this enabled by default, since it shows what part of the file is not actively used and will not have references indexed.

I'm happy to merge the type fix, but I think this should stay on by default.

wyuenho commented 6 years ago

it shows what part of the file is not actively used and will not have references indexed

This makes no sense, why can't I develop the "inactive" code as regular code? Inactive region makes sense when you are viewing or exploring, not editing. An All-or-nothing global option doesn't make sense here.

jacobdufault commented 6 years ago

You can - there's a tradeoff, should you show the user what is indexed by cquery or should you show them the code as normal? I think the more useful tradeoff for the average user is to show what is indexed by cquery. This is what IDEs I'm familiar with (eclipse, visual studio, clion) do by default as well. It can be customized for those who feel otherwise.

wyuenho commented 6 years ago

There are IDEs that don't index everything? I've never seen one TBH.

I'm not even sure why this feature exists. This appears to me as something used for eliminating dead code by the compiler front end that just got exposed.

On 11 Aug 2018, at 1:16 am, Jacob Dufault notifications@github.com wrote:

You can - there's a tradeoff, should you show the user what is indexed by cquery or should you show them the code as normal? I think the more useful tradeoff for the average user is to show what is indexed by cquery. This is what IDEs I'm familiar with (eclipse, visual studio, clion) do by default as well. It can be customized for those who feel otherwise.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

sebastiencs commented 6 years ago

I agree that this should stay on by default. When I'm editing c/c++ code I expect my editor to highlight inactive region

jacobdufault commented 6 years ago

I'm not even sure why this feature exists. This appears to me as something used for eliminating dead code by the compiler front end that just got exposed.

This style of programming is quite popular:

#if WIN
void PlatformFunc() {}
#elseif LINUX
void PlatformFunc() {}
#endif
wyuenho commented 6 years ago
#if WIN
void PlatformFunc() {}
#elseif LINUX
void PlatformFunc() {}
#endif

I fail to see why this justifies the existence of an inactive region feature, or for that matter, having it enabled by default. Developing on one platform and then cross-compiling to others is also quite common, I fail to see why commenting out code that is inactive in a particular architecture aids this development workflow.

jacobdufault commented 6 years ago

I fail to see why commenting out code that is inactive in a particular architecture aids this development workflow.

It's useful to see what code is actually being compiled and indexed - sometimes preprocessor definitions are not as trivial as the example above. Like I said, since you don't find this helpful you can turn it off and never think about it again.

I believe, and based on what other tools do by default, that the average user finds this helpful. Beyond that, if this were off by default then people likely will not know it even exists and never turn it on.

There are IDEs that don't index everything? I've never seen one TBH.

I'm not aware of any IDEs that index everything; perhaps cscope and the like do, but IDEs which go beyond regexes do not. You set preprocessor definitions and the IDE indexes based on that.

wyuenho commented 6 years ago

Okay, I've defaulted it back to t

jacobdufault commented 6 years ago

Thanks