pinoaffe / org-vcard

Export and import vCards from within GNU Emacs' Org mode.
61 stars 7 forks source link

Make commands and functions work without prompting #34

Open lovrolu opened 3 years ago

lovrolu commented 3 years ago

Currently org-vcard-export and org-vcard-import know not to prompt the user for the version/language/style depending on the prefix argument. However, they still unconditionally prompt for the source and destination.

Similary, org-vcard-transfer-helper, which I've tried using programmatically to avoid the above, ends up calling either org-vcard-transfer-write or org-vcard-import-parse, both of which unconditionally use read-from-minibuffer in order to read the filename (when exporting to a file).

Would it be possible to modify these functions so that they can be called in a mode where they do no prompting at all and just rely on the values of variables?

Thanks for the neat project! :-)

flexibeast commented 3 years ago

Sorry i've taken so long to respond! i was recently evicted from my previous housing due to the owner selling, and i'm only just now starting to settle into my new place.

Yes, i agree with your proposal. :-) The issue you raised is actually one of a number of infelicities with org-vcard that i'm aware of, and that i'm hoping to start to address in the near future. Unfortunately i can't give any timelines at this point, i'm afraid ....

lovrolu commented 3 years ago

No problem! Sorry to hear about your situation and I hope things are getting better. :-)

As a quick hack, I patched it locally by getting rid of the read-from-minibuffer call. I've thought about submitting it as PR, but a proper fix would require some deeper considerations to make it consistent with the rest of the codebase, rather than just hacking it at a single place or tacking on a new &optional parameter at the end. However, if anyone else is interested, here's the diff:

1 file changed, 5 insertions(+), 5 deletions(-)
org-vcard.el | 10 +++++-----

modified   org-vcard.el
@@ -982,11 +982,11 @@ DIRECTION must be either the symbol 'import or the symbol
                   (buffer-name the-buffer)
                   "'."))))
      ((string= "file" destination)
-      (let ((filename (read-from-minibuffer "Filename? " (cond
-                                                          ((eq 'import direction)
-                                                           org-vcard-default-import-file)
-                                                          ((eq 'export direction)
-                                                           org-vcard-default-export-file)))))
+      (let ((filename (cond
+                        ((eq 'import direction)
+                         org-vcard-default-import-file)
+                        ((eq 'export direction)
+                         org-vcard-default-export-file))))
         (with-temp-buffer
           (insert (string-as-multibyte content))
           (when (file-writable-p filename)
flexibeast commented 2 years ago

i'm finally getting to working through my ICT backlog. :-) i've now added an optional filename argument to the org-vcard-import-parse and org-vcard--transfer-write functions; can you let me know if the changes work for you?

lovrolu commented 2 years ago

Hi!

The way I actually use org-vcard's API is the following (which I extracted from org-vcard-export):

(let* ((style "flat")
       (language "en")
       (version "2.1")
       (org-vcard-default-export-file "~/Desktop/...")
       (org-vcard-default-vcard-21-character-set 'utf-8)
       (org-vcard-styles-languages-mappings ...))
  (org-vcard-transfer-helper "buffer" "file" style language version 'export))

Calling org-vcard-transfer-helper seemed like the proper entrypoint, since it handles getting the content from the appropriate source automatically, and allows me to specify the language, version, style, etc.

On the other hand, org-vcard--transfer-write would require me to somehow manually come up with the content to specify, and it also doesn't allow me to specify the style, language, version, etc. (I would effectively have to replicate whatever the internals of org-vcard already do). Also, the function seems to be internal now (since 2209c34e2e2aea2a9202f0f207880641e6b461a0), so I think I shouldn't rely on calling it directly. :-)

org-vcard-import-parse that you mentioned similarly doesn't fit into my use-case because its job is to do import, when I want export, but I still think it should be adjusted in a similar way, for uniformity purposes.

To me it seems the best way would be to extend org-vcard-transfer-helper with a filename parameter, and then make sure to propagate it all the way down to all of the other functions.

lovrolu commented 2 years ago

By the way, one thing that's a little bit confusing are the differences in the semantics of the various org-vcard-default-... variables, mainly because of the reach/scope they have.

Some of them only specify the initial value to provide in the minibuffer when reading a thing from the user interactively and are not used if org-vcard is used programmatically (through the public functions), while some of them are meant to be global defaults that are never prompted for and cannot be passed as an argument. Concretely:

org-vcard-default-vcard-21-character-set is a global default and affects the encoding used throughout all of org-vcard, but it cannot be specified as an argument to one of the public functions such as org-vcard-transfer-helper, nor is it ever prompted for.

org-vcard-default-{style,language,version} are used by org-vcard-{import,export} only, which propagate it when calling the public function org-vcard-transfer-helper, meaning that if the public function were used directly, those defaults wouldn't be in effect.

org-vcard-default-export-file is similar to the previous, except that filenames are always prompted for, regardless of the public function used (hence the issue :-)).

Maybe I'm missing something and this was done on purpose, but I think it would be a good idea to make things a little bit more uniform, e.g. by making the effects of these default variables hold both in the non-prompting interactive and in the programmatic use-cases. There could then be additional prompting interactive functions (or just additional prefix arguments) which use the defaults only as a suggestion and explicitly prompt the user for the final value. :-)

lovrolu commented 2 years ago

After pulling the latest changes, I bumped into an unrelated issue which I reported separately: #37.