Restart clangd when changing build config, add UI test #2907
Conversation
| const cppRootDir = path.join(rootDir, 'cpp'); | ||
|
|
||
| fs.mkdirSync(cppRootDir); | ||
| fs.writeFileSync(path.join(cppRootDir, 'paul.cpp'), ` |
There was a problem hiding this comment.
clangd is hard-coded to only work with files called paul.cpp.
|
There seem to be an issue with the travis build, but it doesn't seem related to your changes at first glance. |
|
I can have a look at the UI test part. |
simark
force-pushed
the
cpp-restart-clangd
branch
from
90b36ed to
e36b15c
Compare
|
I just rebased and replaced |
simark
force-pushed
the
cpp-restart-clangd
branch
from
d038118 to
26694fc
Compare
|
I think we want eventually to move optional language extensions to their own repositories, like java, cpp. I'm not sure that it is a good idea to couple UI tests in example packages with them. Maybe it is already better to have a dedicated repo for the end to end UI testing of the cpp integration. // cc @svenefftinge |
|
@akosyakov yes, we should extract them. |
|
I have created https://bugs.eclipse.org/bugs/show_bug.cgi?id=539262 |
Yes, that was the intention, to split the cpp extension after the move to Eclipse. Including moving the UI test. And when we do that, I intend to remove the code that skips the test if clangd is not found (it will be a requirement for running the cpp tests). But for these separate extensions to be able to have some UI tests, I think we'll need to have some library that provides these |
| const out = cp.execSync('clangd -version', {encoding: 'utf8'}); | ||
| // Match 'clangd version' at the start of | ||
| // 'clangd version 8.0.0 (trunk 341484) (llvm/trunk 341481)'. | ||
| return out.indexOf('clangd version') == 0; |
There was a problem hiding this comment.
"===" instead of "=="?
There was a problem hiding this comment.
indexOf can only return a number, so == is fine here. Otherwise, I think the linter would have told me.
| */ | ||
| function hasClangd() { | ||
| try { | ||
| const out = cp.execSync('clangd -version', {encoding: 'utf8'}); |
There was a problem hiding this comment.
At least on my Ubuntu machine, installing clangd 8 from the LLVM PPA (package: clang-tools-8), I do not end-up with a "clangd" executable or symlink. Rather the name is "clangd-8", and so I think it will not be detected as installed, here.
There was a problem hiding this comment.
Indeed, for now it's necessary to create that link by hand, see:
https://github.com/theia-ide/theia/blob/master/packages/cpp/README.md
Eventually, it will be done automatically:
http://lists.llvm.org/pipermail/clangd-dev/2018-September/000138.html
simark
force-pushed
the
cpp-restart-clangd
branch
from
26694fc to
b4d7b25
Compare
simark
force-pushed
the
cpp-restart-clangd
branch
from
b4d7b25 to
0247c4d
Compare
See individual patches for more details.