search

Boruch-Baum / emacs-crossword

Play/Download crossword puzzles in Emacs
GNU General Public License v3.0
67 stars 4 forks source link

Prefer curl to wget #3

Closed rnkn closed 3 years ago

rnkn commented 3 years ago

Hello!

Just found your package via Hacker News. Congrats on the frontpage spot!

I have a suggestion that might improve compatibility. wget is not present on non-Linux systems by default. You'll get better compatibility by using curl. Or, if you wanted to be really clever, Emacs can retrieve things online itself using the url library (although I find it a bit inscrutable).

Boruch-Baum commented 3 years ago

Just found your package via Hacker News. Congrats on the frontpage spot!

Thanks! The level of publicity so uickly was unexpected.

I have a suggestion that might improve compatibility. wget is not present on non-Linux systems by default. You'll get better compatibility by using curl. Or, if you wanted to be really clever, Emacs can retrieve things online itself using the url library (although I find it a bit inscrutable).

Good idea. This does put me in the totally unforeseen circumstance of wishing I had a "non-linux" (ahem) system handy to test with.

basil-conto commented 3 years ago

See lisp/gnus/mm-url.el (M-x find-library RET mm-url RET) for a way of handling multiple internal/external retrievers.

Either way, if you allow more than one retriever it should be customisable, and GNU Emacs should probably default to GNU Wget over a non-GNU program, if both are available.

Boruch-Baum commented 3 years ago

With commit 5aaa6ee, the downloader will use 'curl' when 'wget' is unavailable.

basil-conto commented 3 years ago

Thanks. Looks like it's still using wget due to copy pasta though?

https://github.com/Boruch-Baum/emacs-crossword/blob/5aaa6eedeb434baf7aa22f4286a98b5e8542066f/crossword.el#L1569-L1571

rnkn commented 3 years ago

Just to echo Basil, you could likely significantly cut down your lines of code by delegating fetching to the mm-url library, e.g. mm-url-insert-file-contents URL to a temp buffer, then write-file.

Boruch-Baum commented 3 years ago

On 2021-01-19 07:53, Basil L. Contovounesios wrote:

Thanks. Looks like it's still using wget due to copy pasta though?

Is "copy pasta" a typo for something? I'm not up on all the jargon. The intent is to use wget if it's available; otherwise, curl if it's available; otherwise signal an error.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

basil-conto commented 3 years ago

By copy pasta I mean what's probably a copy-paste error. See pull request #5.

Boruch-Baum commented 3 years ago

On 2021-01-19 08:13, Paul W. Rankin wrote:

Just to echo Basil, you could likely significantly cut down your lines of code by delegating fetching to the mm-url library,

When I read Basil's message, I took a first look at the package, and was immediately put off by its documentation. Here it is in its entirety:

;;; Commentary:

;; Some code is stolen from w3 and url packages. Some are moved from ;; nnweb.

;; TODO: Support POST, cookie.

It may be the best thing since ---------, but if it isn't 'discover-able' it might as well not exist. Without better documentation, I need to sit down and read the code first.

e.g. mm-url-insert-file-contents URL to a temp buffer, then write-file.

That's discouraging, because it sounds inefficient compared to using wget/curl to directly download to disk.

-- hkp://keys.gnupg.net CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0

basil-conto commented 3 years ago

It may be the best thing since ---------, but if it isn't 'discover-able' it might as well not exist. Without better documentation, I need to sit down and read the code first.

That's discouraging, because it sounds inefficient compared to using wget/curl to directly download to disk.

FTR, Basil's message never suggested depending on a Gnus sub-package (in fact, Basil would discourage that); it merely pointed to it as an example of precedent.