Fix intellisense errors by moving TerminalApp projects around

[zadjii-msft]

The easiest fix was actually just moving all the source files from
TerminalApp to TerminalApp/lib, where the appropriate pch.h
actually resides.

Closes #6866

[rpjohnst]

FYI in case you are interested in pushing this further- this approach will prevent IntelliSense from actually pre-compiling that header, losing out on that optimization. This means longer pauses after first opening a file, and after making "top-level" edits (i.e. outside the body of a function).

The rule of thumb we (the IntelliSense team) use here is that the build should still work with PCH disabled, and then IntelliSense should automatically work as well. If I understand correctly, then adding lib to TerminalAppLib's include paths shouldn't cause any conflicts (as there are no other header files there), and would allow everything to work.

[miniksa]

FYI in case you are interested in pushing this further- this approach will prevent IntelliSense from actually pre-compiling that header, losing out on that optimization. This means longer pauses after first opening a file, and after making "top-level" edits (i.e. outside the body of a function).

The rule of thumb we (the IntelliSense team) use here is that the build should still work with PCH disabled, and then IntelliSense should automatically work as well. If I understand correctly, then adding lib to TerminalAppLib's include paths shouldn't cause any conflicts (as there are no other header files there), and would allow everything to work.

Neither @zadjii-msft or I can tell whether your suggestion means "use the include path INSTEAD of adding the extra PCH to all of the headers" or "use the include path IN ADDITION TO adding the extra PCH to all of the headers." Can you please clarify, @rpjohnst?

[zadjii-msft]

And you know what, I did try just adding $(OpenConsoleDir)\cascadia\TerminalApp\lib\; to <AdditionalIncludeDirectories> in TerminalApp.vcxproj, and removing the lib/pch.h line from TerminalPage.cpp, but that gave me the same list of Intellisense errors as always. Maybe I'm doing it wrong?

[rpjohnst]

Ah, sorry for the confusion. I meant to suggest using the include path instead of adding the extra #include lines.

And now that you mention that you've tried that, I think I see why it didn't work- #include "pch.h" with "" instead of <> prioritizes the source file's directory over the include search paths, and the wrong PCH happens to be in the source file's directory.

So I tried adding $(OpenConsoleDir)\cascadia\TerminalApp\lib and replacing #include "pch.h" with #include <pch.h> and that seems to be working. (Well, I'm now hitting a different bug where IntelliSense crashes on some coroutines stuff, which TerminalPage.cpp is full of, but at least it's not spitting out errors 🙂)

[zadjii-msft]

Okay, weird. I've tried that too, but now it seems that whenever I open a cpp file with the <pch.h> in it, it will intermittently have intellisense errors in it, until I load up the header for that file. Once intellisense has had a chance to parse the header too, it seems like the errors in the cpp file go away as well.

I have no idea if that's expected or not.

[rpjohnst]

Hm, that's definitely not expected. A guess at what's happening- perhaps the cpp file you open finds the wrong pch.h, while the header finds the correct one. IntelliSense could then build a working PCH in response to opening the header, which could then be picked up by the cpp file when you return to it.

If so, one thing to double check is the order of the AdditionalIncludeDirectories list. Make sure lib is before the existing ... (Or even replace it, if that doesn't break anything else?) It might also help to temporarily disable PCH use project-wide and make sure the compiler can still find everything.

If that doesn't help, then maybe it's just some leftover state that would go away if you did "Project > Rescan File," restarted VS, and/or deleted the .vs directory. (Fair warning, that will reset a few things like the set of open files; if that's a problem then it may be enough to delete just .vs/<Project>/v16/ipch)

[miniksa]

K well. I was having Intellisense issues and I merged this into my branch and they're gone. Well, Intellisense is still slow but that's par for a WinRT project. So I don't care if there's a better way, this is better than 100% broken as it is now.

[DHowett]

Do we have a spare pch file to delete after this?

[DHowett]

It might be even easier to move the lib project into the TerminalApp/ folder and make a dll project that just dllizes it

[DHowett]

in fact, i might block to discuss that

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[DHowett]

does the dll project even need the ClIncludes? Can it live w/o them?

[zadjii-msft]

probably?

[ghost]

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.