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

Comments

Projects
None yet
2 participants
@yyoncho

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

This comment has been minimized.

Show comment
Hide comment
@cpitclaudel

cpitclaudel Sep 24, 2018

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.

Owner

cpitclaudel commented Sep 24, 2018

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

This comment has been minimized.

Show comment
Hide comment
@yyoncho

yyoncho Sep 24, 2018

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

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

This comment has been minimized.

Show comment
Hide comment
@cpitclaudel

cpitclaudel Sep 24, 2018

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.

Owner

cpitclaudel commented Sep 24, 2018

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

This comment has been minimized.

Show comment
Hide comment
@yyoncho

yyoncho 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?

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

This comment has been minimized.

Show comment
Hide comment
@cpitclaudel

cpitclaudel Sep 24, 2018

Owner

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

Owner

cpitclaudel commented Sep 24, 2018

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

@yyoncho

This comment has been minimized.

Show comment
Hide comment
@yyoncho

yyoncho 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.

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

This comment has been minimized.

Show comment
Hide comment
@cpitclaudel

cpitclaudel Sep 25, 2018

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?

Owner

cpitclaudel commented Sep 25, 2018

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

This comment has been minimized.

Show comment
Hide comment
@yyoncho

yyoncho 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

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

This comment has been minimized.

Show comment
Hide comment
@cpitclaudel

cpitclaudel Sep 26, 2018

Owner

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

Owner

cpitclaudel commented Sep 26, 2018

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

@yyoncho

This comment has been minimized.

Show comment
Hide comment
@yyoncho

yyoncho 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) .

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