Page MenuHomePhabricator

[clangd] Fix the stale documentation about background indexing.
ClosedPublic

Authored by hokein on Aug 29 2019, 4:24 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein created this revision.Aug 29 2019, 4:24 AM

Thanks for updating this! Pinging @sammccall since he is also working on writing some release docs, might be helpful to bring this to your attention.

clang-tools-extra/docs/clangd/Installation.rst
358 ↗(On Diff #217834)

I don't think it is necessary to mention the version. It could be better to directly start with

Clangd builds an incremental index over files listed in the compilation database.
360 ↗(On Diff #217834)

instead of enables let's say improves ?

361 ↗(On Diff #217834)

drop the global

362 ↗(On Diff #217834)

let's mention it will only use your idle cores to build an index and total amount of cores it will use can be limited to X by passing -j=X to clangd

363 ↗(On Diff #217834)

that's not necessarily true, we also save at home directory, and I don't think it is relevant for installation. maybe rather drop it?

364 ↗(On Diff #217834)

s/canb/can
s/disable/disabled

365 ↗(On Diff #217834)

instead of if it's disabled...

Note that, disabling background-index will limit clangd's knowledge about your codebase to
files you are currently editing.
369 ↗(On Diff #217834)

I don't think it is relevant for installation section again. But let's make the you probably don't need this a seperate line and make it bold or red ?

hokein updated this revision to Diff 221508.Sep 24 2019, 4:33 AM
hokein marked 8 inline comments as done.

Address comments.

hokein updated this revision to Diff 221509.Sep 24 2019, 4:35 AM

fix a typo

hokein added inline comments.Sep 24 2019, 4:35 AM
clang-tools-extra/docs/clangd/Installation.rst
363 ↗(On Diff #217834)

I think it is important to mention this. For most cases, .clangd/index is in in the project root, right?

kadircet added inline comments.Sep 24 2019, 5:44 AM
clang-tools-extra/docs/clangd/Installation.rst
363 ↗(On Diff #217834)

yes, but still if you are planning to mention this please also mention something like: Index shards for common headers like standard library will be stored in $HOME/.clangd/index

365 ↗(On Diff #217834)

seems to be the same?

hokein updated this revision to Diff 221523.Sep 24 2019, 5:50 AM
hokein marked 2 inline comments as done.

address comments.

clang-tools-extra/docs/clangd/Installation.rst
365 ↗(On Diff #217834)

oops, I forgot this comment.

kadircet accepted this revision.Sep 25 2019, 1:18 AM

thanks, lgtm!

clang-tools-extra/docs/clangd/Installation.rst
364 ↗(On Diff #221523)

s/shareds/shards

This revision is now accepted and ready to land.Sep 25 2019, 1:18 AM
hokein updated this revision to Diff 221687.Sep 25 2019, 1:24 AM
hokein marked an inline comment as done.

Fix a typo.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2019, 1:25 AM