Merge lp:~stocks29/do-plugins/scripts into lp:do-plugins

Proposed by Bob Stockdale
Status: Work in progress
Proposed branch: lp:~stocks29/do-plugins/scripts
Merge into: lp:do-plugins
To merge this branch: bzr merge lp:~stocks29/do-plugins/scripts
Reviewer Review Type Date Requested Status
Alex Launi (community) Disapprove
Do Plugins Team Pending
Review via email: mp+1494@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Bob Stockdale (stocks29) wrote :

Added a plugin to allow raw text to be passed to perl script file items. Any output of the perl script is displayed as a notification.

Revision history for this message
Alex Launi (alexlauni) wrote :

Why is this useful? Maybe I'm missing something but it only handles perl scripts, why not all scripts? Could you just modify the Run in Terminal action to handle parameters? Also, the Name property should be an action, not a name. Yours is "Perl scripts..." something like, "run script" would be more appropriate. line 82 why do you do @"perl"? perl has no special chars in it... My last question is why is the output going to a notification? Generally when a script gives output, it's meaningful and you would want to do something with it. You can't copy from a notification and it may be so long you can't read it. You should return it as a text item so it can be copied to the clipboard or something.

review: Disapprove (community)
Revision history for this message
Bob Stockdale (stocks29) wrote :

On Mon, Nov 10, 2008 at 5:02 PM, Alex Launi <email address hidden> wrote:

> Vote: Disapprove community
> Why is this useful?

There are many things that integrate nicely via scripts in quicksilver and I
was attempting to duplicate that functionality as a plugin. For example, if
you place a script in a specific folder you can then pass raw text to the
script which is interpreted as command line arguments to whichever script
you select. Any output from the script is then returned as raw text.
Examples:

http://users.physik.fu-berlin.de/~goerz/blog/tag/quicksilver/

http://dcortesi.com/2008/01/12/adium-quicksilver-script/

http://worldairmaillinks.com/post/4692422/hiveminder-to-do-list-via-quicksilver

> Maybe I'm missing something but it only handles perl scripts, why not all
> scripts?

Nope, your not missing anything. I personally am a perl developer, so most
of my scripts are in perl and hence that is what I was working with for
development. I was eventually going to make it work with all scripts -- just
hadn't gotten around to that yet althought I believe it will only be a one
line change.

> Could you just modify the Run in Terminal action to handle parameters?

Well, I could, but it is nice to see the scripts as possible actions. I was
also thinking about having the scripts sorted by language so it is easier to
find what you are looking for. Initially I wanted all languages to just show
up as possible actions, but I could not figure that out hence "Perl scripts
..."

> Also, the Name property should be an action, not a name.
>
Yours is "Perl scripts..." something like, "run script" would be more
> appropriate.

I agree.

> line 82 why do you do @"perl"? perl has no special chars in it...

Sorry, thats me being a n00b to the language. Didn't know that the @ wasn't
require for such a statement -- I saw it in an example.

> My last question is why is the output going to a notification?
>
Generally when a script gives output, it's meaningful and you would want to
> do something with it. You can't copy from a notification and it may be so
> long you can't read it. You should return it as a text item so it can be
> copied to the clipboard or something.

Ha, you actually answered a question about quicksilver for me -- that is,
why does the script output return to a new text item. For my uses, it always
returns some notification message to me, which I prefered to go to a
notification. I would like to implement this so it could go either way...
any ideas on how to do this. I have a preliminary ideas, but nothing solid
yet.

> --
> https://code.launchpad.net/~stocks29/do-plugins/scripts/+merge/1494<https://code.launchpad.net/%7Estocks29/do-plugins/scripts/+merge/1494>
> You are subscribed to branch lp:~stocks29/do-plugins/scripts.
>

--
Bob Stockdale
<email address hidden>

Revision history for this message
Alex Launi (alexlauni) wrote :

On Mon, Nov 10, 2008 at 7:48 PM, Bob Stockdale
<email address hidden>wrote:

> > Maybe I'm missing something but it only handles perl scripts, why not all
> > scripts?
>
> Nope, your not missing anything. I personally am a perl developer, so most
> of my scripts are in perl and hence that is what I was working with for
> development. I was eventually going to make it work with all scripts --
> just
> hadn't gotten around to that yet althought I believe it will only be a one
> line change.

I would make this change sooner rather than later

> > Could you just modify the Run in Terminal action to handle parameters?
>
> Well, I could, but it is nice to see the scripts as possible actions. I was
> also thinking about having the scripts sorted by language so it is easier
> to
> find what you are looking for. Initially I wanted all languages to just
> show
> up as possible actions, but I could not figure that out hence "Perl scripts
> ..."

What do you mean have the scripts show up as actions? You don't have
ItemSource so they would just show up as file items meaning you'd have to
modify the Files & Folders plugin to index the scripts folder. I don't think
I approve of your plugin messing around with the F&F's config, but if you
could find a good way I might allow it.

> > My last question is why is the output going to a notification?
> >
> Generally when a script gives output, it's meaningful and you would want to
> > do something with it. You can't copy from a notification and it may be so
> > long you can't read it. You should return it as a text item so it can be
> > copied to the clipboard or something.
>
>
> Ha, you actually answered a question about quicksilver for me -- that is,
> why does the script output return to a new text item. For my uses, it
> always
> returns some notification message to me, which I prefered to go to a
> notification. I would like to implement this so it could go either way...
> any ideas on how to do this. I have a preliminary ideas, but nothing solid
> yet.

to return a new text item all you have to do is "return new TextItem
("message");" If you wanted to make it configurable you'd need to implement
IConfigurable, but I wouldn't do that. IMO options kill software, just make
a sane choice (which would be returning the output as a text item).

--
--Alex Launi

Revision history for this message
Bob Stockdale (stocks29) wrote :

Alex,

I took care of the items we discussed.

-Bob

On Mon, Nov 10, 2008 at 8:51 PM, Alex Launi <email address hidden> wrote:

> On Mon, Nov 10, 2008 at 7:48 PM, Bob Stockdale
> <email address hidden>wrote:
>
> > > Maybe I'm missing something but it only handles perl scripts, why not
> all
> > > scripts?
> >
> > Nope, your not missing anything. I personally am a perl developer, so
> most
> > of my scripts are in perl and hence that is what I was working with for
> > development. I was eventually going to make it work with all scripts --
> > just
> > hadn't gotten around to that yet althought I believe it will only be a
> one
> > line change.
>
>
> I would make this change sooner rather than later
>
>
>
> > > Could you just modify the Run in Terminal action to handle parameters?
> >
> > Well, I could, but it is nice to see the scripts as possible actions. I
> was
> > also thinking about having the scripts sorted by language so it is easier
> > to
> > find what you are looking for. Initially I wanted all languages to just
> > show
> > up as possible actions, but I could not figure that out hence "Perl
> scripts
> > ..."
>
>
> What do you mean have the scripts show up as actions? You don't have
> ItemSource so they would just show up as file items meaning you'd have to
> modify the Files & Folders plugin to index the scripts folder. I don't
> think
> I approve of your plugin messing around with the F&F's config, but if you
> could find a good way I might allow it.
>
>
>
>
> > > My last question is why is the output going to a notification?
> > >
> > Generally when a script gives output, it's meaningful and you would want
> to
> > > do something with it. You can't copy from a notification and it may be
> so
> > > long you can't read it. You should return it as a text item so it can
> be
> > > copied to the clipboard or something.
> >
> >
> > Ha, you actually answered a question about quicksilver for me -- that is,
> > why does the script output return to a new text item. For my uses, it
> > always
> > returns some notification message to me, which I prefered to go to a
> > notification. I would like to implement this so it could go either way...
> > any ideas on how to do this. I have a preliminary ideas, but nothing
> solid
> > yet.
>
>
> to return a new text item all you have to do is "return new TextItem
> ("message");" If you wanted to make it configurable you'd need to implement
> IConfigurable, but I wouldn't do that. IMO options kill software, just make
> a sane choice (which would be returning the output as a text item).
>
> --
> --Alex Launi
>
> https://code.launchpad.net/~stocks29/do-plugins/scripts/+merge/1494<https://code.launchpad.net/%7Estocks29/do-plugins/scripts/+merge/1494>
> You are subscribed to branch lp:~stocks29/do-plugins/scripts.
>

--
Bob Stockdale
<email address hidden>

lp:~stocks29/do-plugins/scripts updated
270. By Bob Stockdale <bobby@stocksxps>

Started making changes proposed by Alex.

271. By Bob Stockdale <bobby@stocksxps>

Corrected Do.File dependency name in xml config.

Revision history for this message
Alex Launi (alexlauni) wrote :

You're still matching on file extensions, this is no good. What if you just indexed all of the files in a user specified scrips directory, ~/Scripts or something. The list of possible script types is too long to enumerate them all like that, you need a better solution. What about ruby, LOLcode, lua, boo, lisp, php, and the other 10000 scriptings languages that are out there, your plugin should not care what language the script is written in.

You still have the PerlScriptAction.cs file, You should remove this.

line 109 in ScriptAction you have a huge long error message, you should break that into a couple of lines, try not to exceed 120 chars per line.

the block starting at 118 can be replaced with a WaitForExit () call, http://msdn.microsoft.com/en-us/library/system.diagnostics.process.waitforexit.aspx

line 122 rather than p_result == string.Empty, use if (string.IsNullOrEmpty(p_result)) { }, I would do a ternary operation there, it'd be a bit more concise.

Rather than FileItem you should be using IFileItem, p_result should be pResult (but really should be something more descriptive. scriptOutput?).

Why do you depends on Do.File in your addin.xml? FileItem and IFileItem should be provided by core.

Looking good, just those couple of minor issues.

Revision history for this message
Matthew Frizelle (gleebtorin) wrote :

"You're still matching on file extensions, this is no good. What if you just indexed all of the files in a user specified scrips directory, ~/Scripts or something. The list of possible script types is too long to enumerate them all like that, you need a better solution. What about ruby, LOLcode, lua, boo, lisp, php, and the other 10000 scriptings languages that are out there, your plugin should not care what language the script is written in."

Agreed. If you're going to be grouping scripts per type, might I suggest using the binary name as listed in the shebang line? This would result in most, if not all, of the scripts having a human-readable indicator of script type.

Revision history for this message
Bob Stockdale (stocks29) wrote :

I'm starting to make the suggested changes but I have a few questions I was
hoping someone might be able to help me with.

Moving down the road of grabbing all of the files (scripts) in a specific
directory, do I need to create an ItemSource to accomplish this? I'm having
a little trouble following the design... is any API documentation available?

Thanks,
-Bob

On Tue, Feb 17, 2009 at 11:43 AM, Matthew Frizelle <email address hidden>wrote:

> "You're still matching on file extensions, this is no good. What if you
> just indexed all of the files in a user specified scrips directory,
> ~/Scripts or something. The list of possible script types is too long to
> enumerate them all like that, you need a better solution. What about ruby,
> LOLcode, lua, boo, lisp, php, and the other 10000 scriptings languages that
> are out there, your plugin should not care what language the script is
> written in."
>
> Agreed. If you're going to be grouping scripts per type, might I suggest
> using the binary name as listed in the shebang line? This would result in
> most, if not all, of the scripts having a human-readable indicator of script
> type.
> --
> https://code.launchpad.net/~stocks29/do-plugins/scripts/+merge/1494<https://code.launchpad.net/%7Estocks29/do-plugins/scripts/+merge/1494>
> You are subscribed to branch lp:~stocks29/do-plugins/scripts.
>

--
Bob Stockdale
<email address hidden>

Revision history for this message
Alex Launi (alexlauni) wrote :

If you want to index a directory yes, but you could also just use the items provided by the file item source.

p.s. sorry it took me a month to respond

Revision history for this message
Bob Stockdale (stocks29) wrote :

Thanks for the info. Not sure if you saw my other question, but are there
any API docs I could use as a reference?

On Fri, May 1, 2009 at 3:46 PM, Alex Launi <email address hidden> wrote:

> If you want to index a directory yes, but you could also just use the items
> provided by the file item source.
>
> p.s. sorry it took me a month to respond
> --
> https://code.launchpad.net/~stocks29/do-plugins/scripts/+merge/1494<https://code.launchpad.net/%7Estocks29/do-plugins/scripts/+merge/1494>
> You are subscribed to branch lp:~stocks29/do-plugins/scripts.
>

--
Bob Stockdale
<email address hidden>

Revision history for this message
Alex Launi (alexlauni) wrote :

An incomplete one that might be helpful-
http://do.davebsd.com/wiki/index.php?title=Writing_Plugins

--
--Alex Launi

Unmerged revisions

271. By Bob Stockdale <bobby@stocksxps>

Corrected Do.File dependency name in xml config.

270. By Bob Stockdale <bobby@stocksxps>

Started making changes proposed by Alex.

269. By Bob Stockdale <bobby@stocksxps>

Removed an un-necessary line of code.

268. By Bob Stockdale <bobby@stocksxps>

Added some more helpful diagnostics if unable to start process.

267. By Bob Stockdale <bobby@stocksxps>

Added notifications for scripts plugin.

266. By Bob Stockdale <bobby@stocksxps>

Commented out the notification line.

265. By Bob Stockdale

Added framework for notifications of script output.

264. By Robert Stockdale <bobby@stocks>

Missed a file that was updated with the scripts plugin.

263. By Robert Stockdale <bobby@stocks>

Initial checkin of Scripts project and PerlScriptAction plugin.

Subscribers

People subscribed via source and target branches