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

Lenses support #12

Open
yyoncho opened this issue Sep 24, 2018 · 10 comments
Open

Lenses support #12

yyoncho opened this issue Sep 24, 2018 · 10 comments

Comments

@yyoncho
Copy link

yyoncho commented Sep 24, 2018

I am working on lsp-mode and I want to use the library for displaying lenses (https://camo.githubusercontent.com/348d682e4a4cc1f84e8571a9488c2087ae754c88/68747470733a2f2f692e696d6775722e636f6d2f437846506f50472e706e67) this would require changes to quick-peek to support that option(e. g. removing the spacers). Are you fine with that approach or you would prefer to fork it?

@cpitclaudel
Copy link
Owner

I use quick-peek in company-coq to display info in the style that you call lenses, so I'm all for supporting your use case too :) IOW, I'd prefer patches over a fork.

@yyoncho
Copy link
Author

yyoncho commented Sep 24, 2018

I might be missing something but there is no reference to quick-peek in https://github.com/cpitclaudel/company-coq ?

@cpitclaudel
Copy link
Owner

That's because the code from quick-peek was originally extracted from company-coq, and I haven't done the refactoring to make company-coq use the separately packaged version yet.

@yyoncho
Copy link
Author

yyoncho commented Sep 24, 2018

Ok, just one more question - as far as I can see quick-peek--insert-spacer is called unconditionally - do I miss something or quick-peek as it is does not support what I am calling lenses?

@cpitclaudel
Copy link
Owner

Maybe I'm misunderstanding what you mean by lenses? :)

@yyoncho
Copy link
Author

yyoncho commented Sep 25, 2018

I believe that the following changes are needed (unless I am missing something).

  1. Provide option to display the overlays without the spacer since they take a lot of space.
  2. Optionally allow displaying multiple overlays on the same line.

@cpitclaudel
Copy link
Owner

Provide option to display the overlays without the spacer since they take a lot of space.

Can you show a screenshot? Normally the spacer should be very thin on graphic displays; maybe there's a bug?

@yyoncho
Copy link
Author

yyoncho commented Sep 26, 2018

No, they are fine if you want to display one item but they introduce a lot of noise if you show them on a lot of places in the file. Note that lenses could be visible all the time.

selection_001
vs
selection_002

@cpitclaudel
Copy link
Owner

OK, I'm sold :) Do you want to prepare a pach? We could have a dynamic variable for this.

@yyoncho
Copy link
Author

yyoncho commented Sep 26, 2018

There are a few more things that I have to do before that, then I will provide a patch. Also, I have to figure out whether I have to support multiple lenses on a single line which may complicate the code a bit. Also, choosing overlays kind of depends on noctuid/link-hint.el#24 (comment) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants