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

Add recipes/cquery #5235

Merged
merged 1 commit into from Jan 22, 2018

Conversation

Projects
None yet
2 participants
@MaskRay
Contributor

MaskRay commented Jan 12, 2018

Brief summary of what the package does

emacs-cquery is a client for cquery, a low-latency language server supporting multi-million line C++ code-bases, powered by libclang.

It leverages lsp-mode, but also provides some cquery extensions to LSP:

  • semantic highlighting
  • inactive region (e.g. a #if false region)
  • cross references: $cquery/base $cquery/callers $cquery/derived $cquery/vars

Thus the name does not start with lsp-.

Direct link to the package repository

https://github.com/cquery-project/emacs-cquery

Your association with the package

Contributor

Relevant communications with the upstream package maintainer

cquery.el @topisani

cquery @jacobdufault

Checklist

Please confirm with x:

@purcell

This comment has been minimized.

Member

purcell commented Jan 21, 2018

Thanks. Quick feedback:

  • Don't define a face that has only foreground or background. Either :inherit a built-in face by default (preferred), or define complete face variants for both light and dark backgrounds.
  • If exposing variables as custom variables, then "." is not sufficient documentation for users to figure out how they should use them. :-) Same goes for all the other functions with that docstring. It doesn't take long to write a short description.
  • Would it make sense to add an ;;;#autoload cookie for the lsp-cquery-enable function?
@MaskRay

This comment has been minimized.

Contributor

MaskRay commented Jan 22, 2018

Changed to

(defface cquery-inactive-region-face
  '((t :inherit shadow))
  "The face used to mark inactive regions."
  :group 'cquery)

(defface cquery-code-lens-face
  '((t :inherit shadow))
  "The face used for code lens overlays."
  :group 'cquery)
  • Language server protocol code lens is still a brand new idea to many users. I'm not certain how to pick the best default color.
  • Added more document strings to functions and variables.
  • lsp-cquery-enable is a function generated by the macro (lsp-define-stdio-client ....), so there is no entity to add ;;;#autoload. Similar projects like https://github.com/emacs-lsp/lsp-rust/blob/master/lsp-rust.el also do not use any ;;;#autoload.

@topisani

@purcell

This comment has been minimized.

Member

purcell commented Jan 22, 2018

Language server protocol code lens is still a brand new idea to many users. I'm not certain how to pick the best default color.

True. Hopefully themes will add support over time.

lsp-cquery-enable is a function generated by the macro (lsp-define-stdio-client ....), so there is no entity to add ;;;#autoload

It's still possible to add autoload comments, actually: you just have to include the signature, e.g. ;;;###autoload (autoload 'lsp-cquery-enable "cquery")

Anyway, I'll go ahead and merge this now - thanks!

@purcell purcell merged commit 3cd3bff into melpa:master Jan 22, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MaskRay MaskRay deleted the MaskRay:cquery branch Jan 22, 2018

Ambrevar added a commit to Ambrevar/melpa that referenced this pull request Feb 13, 2018

waymondo added a commit to waymondo/melpa that referenced this pull request Feb 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment