charlesroelli / org-board

Org mode's web archiver.
GNU General Public License v3.0
279 stars 18 forks source link

Cannot archive recursively #9

Open JanPappert opened 7 years ago

JanPappert commented 7 years ago

Hey,

I tried the example in the readme:

** TODO Linkers (20-part series) :PROPERTIES: :URL: http://a3f.at/lists/linkers :WGET_OPTIONS: --recursive -l 1 :END:

However, I am presented with the following error:


wget: --level: Invalid number ‘--recursive’.

/usr/bin/wget --directory-prefix=/home/user/Desktop/data/fb/6116da-9306-4f66-9daf-68199290e668/2017-03-27T01:33:16Z// "-e robots=off" --page-requisites --adjust-extension --convert-links 1 -l --recursive http://a3f.at/lists/linkers exited abnormally with code 2

Archiving without the WGET_OPTIONS does work, however. Recursively archiving with wget in the terminal works as well.

I should note that I also got the following error when running org-board-keymap: Autoloading failed to define function org-board-keymap

Hoping to find some help here, the package itself looks promising.

Cheers

charlesroelli commented 7 years ago

Hi, thanks for submitting this issue and for trying out org-board.

Is it possible that you switched the "1" (one) and the lowercase "L" in the options? Looking again at the example, it is a bit confusing. I will change it to "--layers" (the full form of the option) to remove the ambiguity.

I'm also not sure why org-board-keymap is getting autoloaded; I will have to look into that in more detail.

On 27 Mar 2017, at 01:39, JanPappert notifications@github.com wrote:

Hey,

I tried the example in the readme:

** TODO Linkers (20-part series) :PROPERTIES: :URL: http://a3f.at/lists/linkers :WGET_OPTIONS: --recursive -l 1 :END:

However, I am presented with the following error:

wget: --level: Invalid number ‘--recursive’.

/usr/bin/wget --directory-prefix=/home/user/Desktop/data/fb/6116da-9306-4f66-9daf-68199290e668/2017-03-27T01:33:16Z// "-e robots=off" --page-requisites --adjust-extension --convert-links 1 -l --recursive http://a3f.at/lists/linkers exited abnormally with code 2 Archiving without the WGET_OPTIONS does work, however. Recursively archiving with wget in the terminal works as well.

I should note that I also got the following error when running org-board-keymap: Autoloading failed to define function org-board-keymap

Hoping to find some help here, the package itself looks promising.

Cheers

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

JanPappert commented 7 years ago

I just copied it so I suppose that I did not do the ol' switcheroo. I'll just post the output of the org-board-wget-call buffer since it's not the same for all variations:

** TODO Linkers (20-part series) :PROPERTIES: :URL: http://a3f.at/lists/linkers :WGET_OPTIONS: --recursive -l 1 :END:

yields

wget: --level: Invalid number ‘--recursive’.

/usr/bin/wget --directory-prefix=/home/jpappert/Desktop/data/95/f5272c-c122-4ee2-af43-4f7c4b821e92/2017-03-27T15:11:39Z// "-e robots=off" --page-requisites --adjust-extension --convert-links 1 -l --recursive http://a3f.at/lists/linkers exited abnormally with code 2

while

** TODO Linkers (20-part series) :PROPERTIES: :URL: http://a3f.at/lists/linkers :WGET_OPTIONS: --recursive -layers 1 :END:

yields

wget: --level: Invalid number ‘ayers’.

/usr/bin/wget --directory-prefix=/home/jpappert/Desktop/data/25/aad3e9-9607-433c-a47d-cef3f8724f7c/2017-03-27T15:15:23Z// "-e robots=off" --page-requisites --adjust-extension --convert-links 1 -layers --recursive http://a3f.at/lists/linkers exited abnormally with code 2

I really hope we can resolve this. Archiving bookmarks like this sounds great! It should be noted that I installed it using melpa, perhaps it would be better if I clone it from github and add it to my loadpath?

charlesroelli commented 7 years ago

Turns out I introduced this bug in commit 3ddccc04. I accidentally reversed the list of WGET_OPTIONS while processing it, which caused the undesirable behavior. Please do clone the latest version from here and try it out.

Also, I realized that the suggested example will not natively recurse into the list of sites given by the URL property, since I removed the behavior of `wget' spanning multiple hosts by default (it can result in unnecessarily thorough archives of certain sites). So I have added it back into the example to make it work. Please try out the new example instead.

JanPappert commented 7 years ago

Hey,

your commit solves the issue for me, the example allows me to recursively archive websites. Which means this package just earned a star. :)

I should note that the org-board-keymap issue remains (though it isn't a big deal for me, I just added the keybindings I value in use-package).

Another suggestion: While I suppose that some people use eww, I would guess that most users still use other browsers. Consider switching the default behaviour to org-board-default-browser 'system in order to appeal to a larger audience.

charlesroelli commented 7 years ago

Glad to hear that the main issue is fixed. It shows that I should really write some tests.

I've made the definition for org-board-keymap more orthodox now, and it should be picked up by the autoload generation. Could you please test the new version? It's commit 405bfd63, version 1018. Thank you.

JanPappert commented 7 years ago

Hey, so I played around with the new commit: Opening archives with eww seems really slow for some reason, it froze my emacs for like 20 seconds.. Concerning the keymap: If I put (global-set-key (kbd "C-c o") 'org-board-keymap) into my dotfile I will receive the error

Wrong type argument: commandp, org-board-keymap

If I change it to (global-set-key(kbd "C-c o") (lambda () (interactive) (org-board-keymap))) I receive

Symbol's function definition is void: org-board-keymap

Cheers

charlesroelli commented 7 years ago

About org-board-keymap, it's a typo in the commentary! Sorry about that. It should be

(global-set-key (kbd "C-c o") org-board-keymap)

(i.e. org-board-keymap should be unquoted, since we want to set the key C-c o to the value of the variable naming the keymap, and not a symbol referencing the variable). I've fixed this in commit 1daf7bb.

The Emacs browser works reasonably fast for me, but what can take a while is searching the archive to find the correct file to open when you run org-board-open. The code that does this is not very smart. Could you please profile a slow run of it, using the instructions here: https://www.gnu.org/software/emacs/manual/html_node/elisp/Profiling.html (you can profile by CPU since memory is unlikely to be an issue for this use case). That should point to the bottleneck in the code, if it's where the problem is.

JanPappert commented 7 years ago

Alright, I tried that but didn't really know how to share the output. Hope this helps; CPU-Profiler-Report.txt

charlesroelli commented 7 years ago

Hm, this looks more like an inefficiency in the Emacs browser, and not in org-board. Does the same slowdown occur when you load the page normally (i.e. via the web)?