Aside: is this debug messaging convention OK? I know it isn't really seen elsewhere in the codebase. My thinking was that sending to cerr is probably more reliable because the IDE has its cout/stdout redirected. It's also easier to scan for relevant messages in the console or a redirected log file rather than the mix of debug/non-debug info in the IDE post window.

I can confirm that this fixes the issue.

Likewise confirmed, selectedString_ is working with this fix.

I'm unclear on the right place to define SC_DEBUG_DOC_MANAGER (these magical things that C++ folks "just know" and the rest of us don't).

@jamshark70 you can just add a #define right before the #ifdef, that's what I was doing. :) I think there's a way to sneakily pass arbitrary preprocessor defines through Cmake, but I haven't got that down yet (it's not -D).

This is an example how a variable that is passed in via -D (fortify) is turned into a definition:

https://github.com/supercollider/supercollider/blob/master/CMakeLists.txt#L285

Right, but as far as arbitrary defines go, AFAICT you have to edit the makefiles to get it to work.

Is this arbitrary enough? :)

https://github.com/supercollider/supercollider/blob/master/CMakeLists.txt#L107

Ah okay, I figured it out. You can add flags to Cmake's compiler invocation flags. Adding the #define was a lot easier for me, haha.

cmake -DCMAKE_C_FLAGS="-D SC_DEBUG_DOC_MANAGER" -DCMAKE_CXX_FLAGS="-D SC_DEBUG_DOC_MANAGER" ..

@bagong I was looking for an arbitrarily named macro, not an arbitrary value for one already recognized by the Cmake project.

Well, I thought you create an option in the cmake script that creates the define with add_definitions if set. That's how its often done. Thereby it's implicitly documented. The line above looks scary to me, doesn't that override other compiler flags? (Hehe, but I'm not sure... just go ahead, I am sure you know what you're doing... Might take others as long as it took you, though, to find that possibility ;))

@bagong

The line above looks scary to me, doesn't that override other compiler flags?

Nope, I just built with it. If you don't provide CMake with a definition they're initialized to the empty string and all other flags are added by appending.

Well, I thought you create an option in the cmake script that creates the define with add_definitions if set. That's how its often done. Thereby it's implicitly documented.

Thus my comment above,

as far as arbitrary defines go, AFAICT you have to edit the makefiles to get it to work.

If this were a global debug flag and not a hyperspecific one with limited usability, that would make sense. Like I said, I'm not aware of any strictly followed conventions in this codebase for adding debugging code. So I'm open to suggestions.

Ah, the AFAICT read like "I am not sure..." to me, my fault.

I think a new block for in the top level CMakeLists.txt for debug stuff would be just fine. The file looks quite chaotic, but it does have blocks of stuff. And many of the individual entries are pretty peripheral... But I am not passionate with this...

I think a new block for in the top level CMakeLists.txt for debug stuff would be just fine.

One already exists.

OK, thanks, I had kind of guess you could define it in the doc manager file, but I wasn't confident enough about that, and time is limited -- if I tried it and it didn't produce logging, I wouldn't have had time to investigate further.

Where, BTW, should this be documented for posterity? (Or, is it enough for long-time C programmers to see the ifdefs and know what to do?)

Where, BTW, should this be documented for posterity? (Or, is it enough for long-time C programmers to see the ifdefs and know what to do?)

The preprocessor is covered in most introductory C++ texts; you have to discuss it when talking about header file include guards. I am certainly not a long-time C programmer, and I've been coding regularly in C++ for less than a year.

Jeez, if I had known that adding debugging code for once would cause this much controversy, I would have just done my five-line commit and called it a day. I can either change the macro usage to NDEBUG, or add a commented-out #define before the macro to hint at the functionality, OR remove the macros altogether and print actual error messages when these commands fail, but I won't spend time documenting this anywhere outside the only file where it's relevant, which is the first file you'd look at if you were trying to debug the document manager anyway. If anyone is trying to fix a problem like this without actually opening the source, well, godspeed...

Ok, sorry, I really didn't mean it to sound like that.

What I meant was, if someone builds from source but does not habitually read the source code, she wouldn't be aware that it's even possible to debug IDE <--> lang IPC. Such as myself... I saw the problem but I didn't know enough to go to the doc manager source.

But you're right, it's impossible to document everything. Some things do just have to be left for user questions.

Ok, sorry, I really didn't mean it to sound like that.

You didn't... ;)

Ok, using qWarning() now instead of cerr, and I got rid of the macro. So these messages will always post to stderr (not the post window) unless you've specifically built to disable Qt logging, which doesn't happen in any currently supported build mode.

The debug-only error reporting are probably an appropriate case for a runtime assert, which is a concept that SC should probably have and be using anyway. I mocked one up a while ago:
https://github.com/scztt/supercollider/tree/topic/line-numbers
https://github.com/scztt/supercollider/blob/topic/line-numbers/common/SC_Assert.h
https://github.com/scztt/supercollider/blob/topic/line-numbers/common/SC_Assert.cpp

So it would look something like:

} catch (std::exception const& e) {
    SC_ASSERT_MSG(false, std::string("DocumentManager could not handle request:") + e.what());
    return;
}

Not particularly important to introduce with this particular commit, but I think it's a pattern we need to introduce at some point.

I'm going to merge this now; it's a hotfix and can be amended later if need be.

@scztt I would totally be in favor of introducing more robust asserts, it would definitely be nice to have if/when I start to clean up some of the language backend.