Majorly update this extension #30

swflint · 2017-07-01T17:10:45Z

This majorly updates org-capture-extension, by doing the following:

Adding an options pane, making it so that users can select which org-capture template to use when capturing.
Updating the method of performing captures, providing a command rather than just a browser action.
Addig store-link functionality
Adding open-source functionality
Enabling the use of both old and new style links
Updating (minorly) the manifest to include an icon.

swflint · 2017-07-01T17:19:35Z

Please note, this and #29 are mutually exclusive. Please choose one or the other.

swflint · 2017-07-01T17:21:38Z

Supercedes #27 and #25.

sprig · 2017-07-03T16:45:20Z

Thanks for your contribution! I'm still going over it.

One thing I'd like to note is that I purposefully did not include the icons in the extension, as I am not sure what their licensing terms are. I felt that it would be safe to use the standard icons in the app store extension as it can be easily replaced if someone asks me to, but the git repository would be much harder to fix (esp. if the request would be regarding previous versions as well).

As such, I would prefer it if the pull request did not have icons in them. Perhaps instructions on where to get them would be appropriate, if I didn't include them in the readme yet.

sprig · 2017-07-03T16:48:13Z

I am also not sure why you removed the EmacsClient.app from the repo

swflint · 2017-07-03T20:53:37Z

I saw no reason to include the EmacsClient.app, as I saw no reference to anywhere.

Icons are likely GPL'd or similarly freely licensed (can be assumed that it's fine to use, see http://orgmode.org/worg/org-faq.html#unicorn).

sprig · 2017-07-03T21:19:52Z

EmacsClient.app is referenced in the readme.
GPL cannot apply to art, AFAIK. Perhaps they have some CC license or something else similar.
I agree that it is reasonable to assume that no one would object to using the icons - this is why I use them for the extension in the chrome/mozilla app stores. However, to add them to the repository I'd want explicit permission or explicit license clause. I'll ask about it on the org mailing list.

store mechanism

swflint · 2017-07-03T22:48:44Z

And EmacsClient.app.zip is added back in.

jabranham · 2017-07-04T05:56:42Z

options.js

+function save_options() {
+    var selTemp = document.getElementById('selTemplate').value;
+    var unselTemp = document.getElementById('unselTemplate').value;
+    var useOldStyleLinks = document.getELementById('useOldstyle').checked;


document.getElementById has a capital L instead of lower case.

Thanks! Fixed

swflint · 2017-07-07T18:42:03Z

Do we have any progress on the licensing of the Unicorn?

sprig · 2017-07-16T23:53:40Z

Sorry - was busy - sent the question just now.
I will continue reviewing the code. As to your question posed (I think) in the other PR - I prefer the smaller PR precisely because it is easier to review a smaller PR compared to a larger one.

nerikj · 2017-07-26T08:56:53Z

I found out that I had to an extra slash in order for things to work in Chrome (same as here: https://github.com/alphapapa/org-protocol-capture-html#chrome).

nerikj · 2017-07-26T09:06:15Z

(The issue is that I'm running Linux of course)

swflint · 2017-07-26T14:20:26Z

@nerikj, Where does the extra slash need to be?

nerikj · 2017-07-26T15:44:49Z

@swflint To get it working, I only needed to add it in createCaptureURL (I'm using new style links but it should affect old style links as well): https://github.com/sprig/org-capture-extension/pull/30/files#diff-a163ccb1fb82b424cf3f4d98ce75b361R15

It needs to be added after the protocol part:

org-protocol:///capture...

swflint · 2017-07-26T16:39:53Z

Fixed it.

nerikj · 2017-07-26T16:41:27Z

Awesome, thanks (and thanks for the PR)!

swflint · 2017-08-05T01:51:19Z

Do we have any info about the logo? Or should I just remove it?

sprig · 2017-08-16T23:05:59Z

There were no answers regarding the logo on the mailing list. I am conflicted on this, but unless I get permission, I think I still would rather not have the logo in the official repo so yes, please remove.

sprig · 2017-08-16T23:13:28Z

@swflint @nerikj The issue with the triple '/' was discussed at length in #16, and instructions regarding it were added to the readme. Please remove that part from the PR.

nerikj · 2017-08-17T12:06:40Z

I'm running Gnome (and don't have any KDE-stuff installed) but get the same issue so maybe it affects certain versions of xdg-open or something like that (I haven't investigated which program it delegates to yet). My point is that it doesn't seems to be only KDE-related.

Anyway, since Linux (xdg-open) is the culprit, I'm fine with the workaround and reverting 725a545.

sprig · 2017-08-18T22:13:53Z

@nerikj ah, that's good to know! It appeared previously to be strictly a KDE problem...
Actually, now that #29 is in, I think it should be very simple to add another option for constructing "///" links. I've opened a ticket (#31) for this issue, and if no one beats me to it, I will add this feature in the near future.

sprig · 2017-08-18T22:30:05Z

@swflint I committed #29 and then added some additions on top of it myself.

Some of the reasons I chose #29 over #30 are;

It is smaller and thus much simpler to review.
It does not require new (user-visible) permissions. Unless there is a good reason (some killer feature), I rather like the fact that currently the extension requires no special permissions. I am open to adding those in principle, but there should be good cause, discussion, and hopefully also more user input.
Some of the added features here seem redundant. For example, I don't see the utility of adding "open-source" functionality, unless I misunderstand what that means. Right now, if I click opt-cmd-i I can already see the page source and more. Why would someone need an extension to do that?
The icons issue.

I am not exactly sure what you mean by "Updating the method of performing captures, providing a command rather than just a browser action".

I do think that adding store-link functionality could be useful. I think it could be nicely implemented in the context of #32 if you or someone else (potentially me) wanted to do it.

Anyway, thanks for this PR and for #29. I hope to read more of your code in the future!

swflint · 2017-08-19T02:55:29Z

So, the new permissions let it perform the various other commands (store link and open source). I added the open-source as an "example" of how to build a new command.

Browser actions and commands behave differently, a command doesn't have the same permissions, and thus using commands for all of them lets a lot of the commands just be a template (just modify it a bit).

The permissions are required for it to be usable with the other features.

sprig · 2017-08-19T03:30:33Z

Sorry, I don't really understand what you are talking about. Could you elaborate please on what you mean by commands and actions, with perhaps an example and also references to the documentation of both?

swflint · 2017-08-19T13:46:40Z

Sure!

An extension can define infinitely many commands, whereas there are three browser action (window, tab, and another that I can't remember). While tab actions confer certain privileges automatically (access to window.), as you can only have one tab action per extension, commands with the tab privilege allow multiple actions (capture, store link, &c).

sprig · 2017-08-19T16:33:20Z

Thanks for the explanation. I am starting to get an idea of what you mean. If I understand correctly, you are referring to [these commands]( https://developer.chrome.com/extensions/commands). If so, what do you want to accomplish by using "commands"? Would users want to accept elevated permissions in order to use them?

swflint · 2017-08-20T00:59:34Z

Yes, that's correct.

We use those to have more than just one action. One command for each action.

The elevated permissions are just for tab data access, nothing more.

ExpHP · 2017-08-20T03:49:59Z

@nerikj: I know I'm just chasing the white rabbit at this point, but I'm dreadfully curious! I'd really like to know what xdg-open is doing in your case!

So, the way I traced xdg-open down to kde-open5 was pretty simple, and took maybe 15 seconds. I just ran this command:

bash -x $(which xdg-open) org-protocol:///capture://L/a/b/c

The output for me is here in this gist. In my case, the output is quite short, and notably, the last line that does anything special is the call to kde-open5.

I would expect the output to be similarly short if it is delegating the job to something else on your end. (on the other hand, if you don't see anything quite so obvious in your trace output, or if your output is significantly longer than mine, then its possible that xdg-open is using its own fallback logic for parsing URLs and handling .desktop files, a.k.a. the open_generic functions)

alphapapa · 2017-08-20T03:53:01Z

@ExpHP Well, on my system, xdg-open ends with:

case "$DE" in
    kde)
    open_kde "$url"
    ;;

And that function looks like this:

open_kde()
{
    if kde-open -v 2>/dev/null 1>&2; then
        kde-open "$1"
    else
        if [ x"$KDE_SESSION_VERSION" = x"4" ]; then
            kfmclient openURL "$1"
        else
            kfmclient exec "$1"
            kfmclient_fix_exit_code $?
        fi
    fi

    if [ $? -eq 0 ]; then
        exit_success
    else
        exit_failure_operation_failed
    fi
}

It also has open_gnome, open_xfce, open_generic_xdg_mime, etc.

sprig · 2017-08-20T06:38:49Z

Fascinating! Please keep us posted with your findings!

nerikj · 2017-08-20T07:25:35Z

@ExpHP This is my output: https://gist.github.com/nerikj/530c64404520b5143d9db6a550caf3bc#file-xdg-open-trace-three-slashes
It seems to end up using gio and works fine.

If I remove a slash (org-protocol://capture...), I get this output: https://gist.github.com/nerikj/530c64404520b5143d9db6a550caf3bc#file-xdg-open-trace-two-slashes and Emacs only opens a buffer named "c".

ExpHP · 2017-08-20T17:45:12Z

I have looked further into this, specifically testing the open_generic function embedded into xdg-open (i.e. the fallback for when it can't find a desktop environment)

I wrote up about my findings in this MD document.

The summary is as follows:

xdg-open itself never deletes the second colon, not even when you let it do all of the work itself. (open_generic)
- if you force it to do all of the work itself, then Exec=_emacs-client %u (no quotes!) or Exec=_emacs-client will produce the correct behavior.
- alas, these still won't work for KDE
gio open and kde-open5 share the exact same colon-eating bug!
- They probably use the same library for something... but what?
There is no solace. Only pain and misfortune.
- xdg-open and kde-open5 disagree about how to handle a double-quoted "%u", or the omission of %u.
- Either the .desktop format is horribly underspecified, or all of its implementations just suck.

Started updating the package

5a2dcaa

sprig mentioned this pull request Jul 3, 2017

Options update #29

Merged

Added in the icon

208641c

swflint force-pushed the master branch from d17e321 to 33684b7 Compare July 3, 2017 22:46

swflint added 3 commits July 3, 2017 17:47

Add in new commands, support for options, update capture and link

5172bb7

store mechanism

Added an extension Icon

8688f73

Add in open-source feature

1142c77

swflint force-pushed the master branch from 33684b7 to 1142c77 Compare July 3, 2017 22:47

sprig mentioned this pull request Jul 3, 2017

Use new org-protocol link style #25

Closed

jabranham reviewed Jul 4, 2017

View reviewed changes

swflint added 2 commits July 4, 2017 08:38

Fixed Typo

9f64a06

Removed extraneous, duplicate code

2286b22

Update capture URLs to use triple slashes

725a545

sprig closed this Aug 18, 2017

ExpHP mentioned this pull request Aug 20, 2017

broken when using xdg-open #16

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Majorly update this extension #30

Majorly update this extension #30

swflint commented Jul 1, 2017

swflint commented Jul 1, 2017

swflint commented Jul 1, 2017

sprig commented Jul 3, 2017

sprig commented Jul 3, 2017

swflint commented Jul 3, 2017

sprig commented Jul 3, 2017

swflint commented Jul 3, 2017

jabranham Jul 4, 2017

swflint Jul 4, 2017

swflint commented Jul 7, 2017

sprig commented Jul 16, 2017

nerikj commented Jul 26, 2017

nerikj commented Jul 26, 2017

swflint commented Jul 26, 2017

nerikj commented Jul 26, 2017 •

edited

Loading

swflint commented Jul 26, 2017

nerikj commented Jul 26, 2017

swflint commented Aug 5, 2017

sprig commented Aug 16, 2017

sprig commented Aug 16, 2017

nerikj commented Aug 17, 2017

sprig commented Aug 18, 2017

sprig commented Aug 18, 2017

swflint commented Aug 19, 2017

sprig commented Aug 19, 2017 via email

swflint commented Aug 19, 2017

sprig commented Aug 19, 2017 via email

swflint commented Aug 20, 2017

ExpHP commented Aug 20, 2017 •

edited

Loading

alphapapa commented Aug 20, 2017 •

edited

Loading

sprig commented Aug 20, 2017

nerikj commented Aug 20, 2017

ExpHP commented Aug 20, 2017 •

edited

Loading

Majorly update this extension #30

Majorly update this extension #30

Conversation

swflint commented Jul 1, 2017

swflint commented Jul 1, 2017

swflint commented Jul 1, 2017

sprig commented Jul 3, 2017

sprig commented Jul 3, 2017

swflint commented Jul 3, 2017

sprig commented Jul 3, 2017

swflint commented Jul 3, 2017

jabranham Jul 4, 2017

Choose a reason for hiding this comment

swflint Jul 4, 2017

Choose a reason for hiding this comment

swflint commented Jul 7, 2017

sprig commented Jul 16, 2017

nerikj commented Jul 26, 2017

nerikj commented Jul 26, 2017

swflint commented Jul 26, 2017

nerikj commented Jul 26, 2017 • edited Loading

swflint commented Jul 26, 2017

nerikj commented Jul 26, 2017

swflint commented Aug 5, 2017

sprig commented Aug 16, 2017

sprig commented Aug 16, 2017

nerikj commented Aug 17, 2017

sprig commented Aug 18, 2017

sprig commented Aug 18, 2017

swflint commented Aug 19, 2017

sprig commented Aug 19, 2017 via email

swflint commented Aug 19, 2017

sprig commented Aug 19, 2017 via email

swflint commented Aug 20, 2017

ExpHP commented Aug 20, 2017 • edited Loading

alphapapa commented Aug 20, 2017 • edited Loading

sprig commented Aug 20, 2017

nerikj commented Aug 20, 2017

ExpHP commented Aug 20, 2017 • edited Loading

nerikj commented Jul 26, 2017 •

edited

Loading

ExpHP commented Aug 20, 2017 •

edited

Loading

alphapapa commented Aug 20, 2017 •

edited

Loading

ExpHP commented Aug 20, 2017 •

edited

Loading