Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix system header search paths on macOS for Objective-C++, too. #1193

Merged
merged 1 commit into from Feb 23, 2019

Conversation

Projects
None yet
4 participants
@s4y
Copy link
Contributor

s4y commented Feb 22, 2019

I've been running into Valloric/YouCompleteMe#303, even though ycmd has a workaround for it. It looks like the issue is just that I'm writing ObjC++ and that change only affects C++.

So, this change makes -x objective-c++ eligible, too.


This change is Reviewable

aarongable pushed a commit to chromium/chromium that referenced this pull request Feb 22, 2019

Revert "[Vim/YCM] Fix missing system headers when YCM was built with …
…its own libclang."

This reverts commit 75a861f.

Reason for revert: It turns out this is an upstream issue. See:

- https://chromium-review.googlesource.com/c/chromium/src/+/1482557
- Valloric/ycmd#1193

Original change's description:
> [Vim/YCM] Fix missing system headers when YCM was built with its own libclang.
> 
> Bug: 932667
> Change-Id: I2eb9965da1eb3f9ad4a5ce103cf6e32d35019e48
> Reviewed-on: https://chromium-review.googlesource.com/c/1476105
> Auto-Submit: Sidney San Martín <sdy@chromium.org>
> Commit-Queue: Sidney San Martín <sdy@chromium.org>
> Commit-Queue: Asanka Herath <asanka@chromium.org>
> Reviewed-by: Asanka Herath <asanka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#632846}

TBR=sdy@chromium.org,asanka@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 932667
Change-Id: I5a684c91723d28470e1d903cd74674ce145b421b
Reviewed-on: https://chromium-review.googlesource.com/c/1483709
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634829}
Add system header search paths on macOS for Objective-C++.
This change just augments 4993964 to
apply to `-x objective-c++` as well as `-x c++` files.
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #1193 into master will decrease coverage by 0.81%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1193      +/-   ##
==========================================
- Coverage   97.28%   96.46%   -0.82%     
==========================================
  Files          95       95              
  Lines        7108     7109       +1     
==========================================
- Hits         6915     6858      -57     
- Misses        193      251      +58
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 22, 2019

Codecov Report

Merging #1193 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1193      +/-   ##
==========================================
+ Coverage   97.28%   97.28%   +<.01%     
==========================================
  Files          95       95              
  Lines        7108     7109       +1     
==========================================
+ Hits         6915     6916       +1     
  Misses        193      193
@s4y

This comment has been minimized.

Copy link
Contributor Author

s4y commented Feb 22, 2019

Hmm, I think the CI failure is unrelated to my change.

@micbou

micbou approved these changes Feb 23, 2019

Copy link
Collaborator

micbou left a comment

Thanks for the PR. :lgtm:

Hmm, I think the CI failure is unrelated to my change.

Yes, it was a flake.

Reviewed 2 of 2 files at r1.
Reviewable status: 1 of 2 LGTMs obtained

@puremourning
Copy link
Collaborator

puremourning left a comment

:lgtm:

Thanks for sending a PR!

Reviewable status: :shipit: complete! 2 of 2 LGTMs obtained

@micbou

This comment has been minimized.

Copy link
Collaborator

micbou commented Feb 23, 2019

@zzbot r+

@zzbot

This comment has been minimized.

Copy link
Collaborator

zzbot commented Feb 23, 2019

📌 Commit 832ee83 has been approved by micbou

@zzbot

This comment has been minimized.

Copy link
Collaborator

zzbot commented Feb 23, 2019

⌛️ Testing commit 832ee83 with merge 6d8ddd5...

zzbot added a commit that referenced this pull request Feb 23, 2019

Auto merge of #1193 - s4y:objcpp-needs-fixed-header-search-paths-too,…
… r=micbou

Fix system header search paths on macOS for Objective-C++, too.

I've been running into Valloric/YouCompleteMe#303, even though ycmd has a workaround for it. It looks like the issue is just that I'm writing ObjC++ and that change only affects C++.

So, this change makes `-x objective-c++` eligible, too.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/1193)
<!-- Reviewable:end -->
@zzbot

This comment has been minimized.

Copy link
Collaborator

zzbot commented Feb 23, 2019

☀️ Test successful - status-appveyor, status-travis
Approved by: micbou
Pushing 6d8ddd5 to master...

@zzbot zzbot merged commit 832ee83 into Valloric:master Feb 23, 2019

9 of 10 checks passed

codecov/changes 4 files have unexpected coverage changes not visible in diff.
Details
ci/circleci: benchmark Your tests passed on CircleCI!
Details
ci/circleci: python2 Your tests passed on CircleCI!
Details
ci/circleci: python3 Your tests passed on CircleCI!
Details
code-review/reviewable 2 of 2 LGTMs obtained
Details
codecov/patch 100% of diff hit (target 97.28%)
Details
codecov/project 97.38% (+0.1%) compared to bb58a43
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@s4y s4y deleted the s4y:objcpp-needs-fixed-header-search-paths-too branch Mar 4, 2019

@micbou micbou referenced this pull request Mar 6, 2019

Merged

[READY] Update ycmd #3339

zzbot added a commit to Valloric/YouCompleteMe that referenced this pull request Mar 12, 2019

Auto merge of #3339 - micbou:update-ycmd, r=Valloric
[READY] Update ycmd

Include the following changes:

 - PR Valloric/ycmd#1180: trigger semantic completion when instructed by server;
 - PR Valloric/ycmd#1184: simplify LSP completer API for starting server;
 - PR Valloric/ycmd#1187: improve Clangd completer initialization;
 - PR Valloric/ycmd#1191: ease Clangd completer initialization;
 - PR Valloric/ycmd#1193: fix system header search paths on macOS for Objective-C++;
 - PR Valloric/ycmd#1194: update Jedi to 0.13.3 and Parso to 0.3.4;
 - PR Valloric/ycmd#1195: ignore identifiers returned by TSServer in JavaScript;
 - PR Valloric/ycmd#1196: update TSServer to 3.3.3333;
 - PR Valloric/ycmd#1197: support -B flag in C-family languages.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/3339)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.