Nvim setup fixes (POSIX Compatbility, Clang includes and Treesitter Querys)#7455
Nvim setup fixes (POSIX Compatbility, Clang includes and Treesitter Querys)#7455pyqt555 wants to merge 1 commit into
Conversation
… the treesitter querys to the ref keyword from addr.
github-actions
Bot
added
the
utilities
chandlerc
left a comment
chandlerc
left a comment
There was a problem hiding this comment.
Thanks for trying to fix up stuff with the neovim support!
I wonder if we want to keep building the tree-sitter integration as part of this, or just focus on using the language server we have now... But a comment inline for now.
| cd utils/tree_sitter | ||
| tree-sitter generate | ||
| clang -o ~/.config/nvim/parser/carbon.so -shared src/parser.c src/scanner.c -I ./src -Os -fPIC | ||
| clang -o ~/.config/nvim/parser/carbon.so -shared src/parser.c src/scanner.c -I "$ROOT" -I ./src -Os -fPIC |
There was a problem hiding this comment.
It's strange that we create the $ROOT variable above, but here we just did cd utils/tree_sitter, and before that we used $PWD. Should we instead set ROOT to $PWD, and do that just before the cd to make it clear this is so we can add -I pointing back at the root of the repo?
There was a problem hiding this comment.
After further concideration i think it would actually be better practice to set $ROOT to 2 levels up from $0,
(like this for example:
SCRIPT_DIR="$(cd -- "$(dirname -- "$0")" && pwd)"
ROOT="$(cd -- "${SCRIPT_DIR}/../.." && pwd)"
)
and just update every path pointing inside the repo to something like this:
"${ROOT}/utils/tree_sitter/queries"
this would allow the script to be executed form anywhere, even outside the repo.
Should i implement this or do you think there is a better way?
2 participants
###Description
This pr addresses several issues in the nvim setup tooling, namely:
2.1) removed -o pipefail. This is not standard posix, and as such will fail on strict posix shells. As there is no pipes in this script it is safe to remove
2.2) added -I "$ROOT" so clang can find all the headers it needs