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

add support for ivy and completing-read #18

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

peterhoeg
Copy link

@peterhoeg peterhoeg commented Mar 22, 2021

This is what I'm using on my side as I don't use helm.

I can definitely do up some documentation, but before I start writing a novel, I want to be sure you're OK with this.

A few changes here:

  1. split out helm support to a separate file
  2. add support for ivy via separate file
  3. add support for completing-read (for use by vertico)
  4. use Cask to run tests
  5. fixes vdirel-contact-fullname fails for entries without N field #10
  6. add support for running tests via make
  7. rename test file to be in line with the buttercup standard structure

I have a change ready to PR for melpa for the helm and ivy packages.

@peterhoeg
Copy link
Author

I haven't touched the tests either - again, I want to be sure you're OK with the direction first.

@DamienCassou
Copy link
Owner

I think what you have done is great! Are you willing to go to the end of it and suggest several packages to melpa?

@DamienCassou
Copy link
Owner

FYI: melpa/melpa#3398 (comment)

@peterhoeg
Copy link
Author

Sure. I've never done any melpa packaging before (I have a few other local elisp packages that probably should be pushed there too), but not a problem

@DamienCassou
Copy link
Owner

Thank you!

This separates the helm specific parts into their own file.
@DamienCassou
Copy link
Owner

Thank you very much for working on this. Please keep me informed.

@peterhoeg
Copy link
Author

peterhoeg commented Sep 2, 2022 via email

@DamienCassou
Copy link
Owner

If I was to add support for using emacs’s completing-read functionality, would you prefer that directly in vdirel.el or should that also move out into a separate file like helm and ivy? I would argue the former since it’s built-in.

👍 for adding the support to vdirel.el file directly.

@peterhoeg peterhoeg changed the title add support for ivy add support for ivy and completing-read May 16, 2023
@peterhoeg
Copy link
Author

oh, and I should really do tests.

@DamienCassou DamienCassou marked this pull request as draft May 16, 2023 19:40
@DamienCassou
Copy link
Owner

@peterhoeg thank you for your work. Please ping me when you feel ready. In the meantime, I converted your PR to draft.

@peterhoeg
Copy link
Author

peterhoeg commented May 17, 2023 via email

@shymega
Copy link

shymega commented Sep 2, 2024

Very interested in seeing this merged. Good work so far!

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

Successfully merging this pull request may close these issues.

vdirel-contact-fullname fails for entries without N field
3 participants