cap-js-community / tree-sitter-cds

CAP CDS grammar for tree-sitter.
Apache License 2.0
16 stars 2 forks source link

Default TREE_SITTER_QUERIES_DIR does not necessarily exist #9

Closed qmacro closed 9 months ago

qmacro commented 9 months ago

Rather than send another PR, I thought I'd raise this here.

To complete the setup of this treesitter mechanism for CDS, the *.scm files need to be copied to a queries/ dir, as done in the second part of setup-nvim-treesitter.sh.

@David-Kunz 's recent change makes sense and moves from a package manager specific queries/ directory to one in the neovim configuration path.

However, $HOME/.config/nvim/queries/ does not necessarily exist, at least not in my exploratory setup.

So perhaps we want to remove this part of the script:

TREE_SITTER_QUERIES_DIR="${TREE_SITTER_QUERIES_DIR:-"$HOME/.config/nvim/queries"}"

if [[ ! -d "${TREE_SITTER_QUERIES_DIR}" ]]; then
  cat << EOF
Could not find 'queries' directory of nvim-treesitter:
  ${TREE_SITTER_QUERIES_DIR}

Did you install nvim-treesitter?
See <https://github.com/nvim-treesitter/nvim-treesitter>
EOF
  exit 1
fi

as I guess it is perhaps only really relevant in the (old) context of a package manager specific location?

Then, after such a removal, the fact that $HOME/.config/nvim/queries/ doesn't always exist would be mitigated by this part anyway:

CDS_DIR="${TREE_SITTER_QUERIES_DIR}/cds"
mkdir -p "${CDS_DIR}"

Thoughts?

David-Kunz commented 9 months ago

Hi @qmacro ,

Oh, I didn't know it doesn't always exist! Yes, I think it makes sense to create it on the fly. @bugwelle , what's your take?

bugwelle commented 9 months ago

I'd say it makes sense to create it on the fly. But IIRC, NeoVim respects the XDG standard, so we may need to check $XDG_CONFIG_HOME or use something like nvim --get-some-config if that exists.

My initial version was something that "works on my machine". After all, that's how this project started :smile:

See https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html


See https://neovim.io/doc/user/starting.html#standard-path :echo stdpath("config")

bugwelle commented 9 months ago

You can get the NeoVim config directory via nvim -c ':echo stdpath("config")' -c ':q' --headless

CONFIG_PATH="$(nvim -c ':echo stdpath("config")' -c ':q' --headless 2>&1)"

bugwelle commented 9 months ago

@qmacro I assumed the directory would be created once https://github.com/nvim-treesitter/nvim-treesitter was installed. I need to update my non-work PC where I'm still using NeoVim 0.6 (quite outdated).

Could you tell me whether the directory exists after you've installed nvim-treesitter?

Regards, Andre

qmacro commented 9 months ago

It does not exist. After a fresh build of my ephemeral Neovim container image which includes not only the actual installation of nvim-treesitter but also some TSInstall action, this is the state of my .config/nvim/:

root@87625fde56e4:~# tree .config/nvim/
.config/nvim/
|-- after
|   `-- plugin
|       |-- dracula.lua
|       |-- harpoon.lua
|       |-- lint.lua
|       |-- lsp-zero.lua
|       |-- lualine.lua
|       |-- mason.lua
|       |-- nvim-cmp.lua
|       |-- telescope.lua
|       `-- treesitter.lua
|-- init.lua
|-- lazy-lock.json
`-- lua
    `-- qmacro
        |-- auto.lua
        |-- init.lua
        |-- lazy.lua
        |-- remap.lua
        `-- set.lua

5 directories, 16 files
bugwelle commented 9 months ago

Thanks. I'll have to test around a bit. I'm not familiar with NeoVim package managers, etc. @David-Kunz If you know more and are interested in this topic? If so, I'd leave it to you :smile: Otherwise, I'll create a few Docker images for different package managers and try to create some semi-automatic test setup.

// Of course, contributions from you, @qmacro, and other people, are very much welcome as well! :smiley:

bugwelle commented 9 months ago

I've prepared #15 : It unconditionally creates the queries directory.

bugwelle commented 9 months ago

15 is merged. As far as I can see, the initial issue is now resolved (unconditionally creating the queries folder)