[SVN] Proposed change to scriptmate.

Luke Daley ld at ldaley.com
Sun May 18 00:32:51 UTC 2008


On 18/05/2008, at 8:52 AM, Allan Odgaard wrote:

> Since the functions are already in a TextMate module, they should  
> not use a ‘tm’ prefix.

Tried that. Calling them `open` and `exec` caused problems. Most  
likely is that I don't understand namespacing in Ruby.

> What is the rationale behind always returning nil as first array  
> member of tmopen?

Backwards compatibility with my_popen3.

> Do we really need both tmexec and tmexec2?

Probably not. So you are saying to always return stdout and stderr?

> Do we need tmexec at all?
>
> Seems :cmd is really the arguments to exec, e.g. could likely be an  
> array, so probably should be :exec instead.
>
> The my_popen3() function in scriptmate.rb should probably be fully  
> removed, seeing how it is just a wrapper for a public function, if  
> we add tm_process.rb.

Sure, if there are no direct uses of it (outside of scriptmate).

> Instead of tmopen/tmexec we probably should call the functions  
> run_async/run_sync to better hint at what they do. Which makes me  
> think that maybe it should be run(:wait = true/false) instead of two  
> functions.

But the return arguments are different: async will return fds and the  
pid, sync will return the actual output on stdout and stderr. I'm not  
a fan of changing the return signature based on input.

Happy to go for run_async/run_sync.

I'll have a look to see if anyone is directly using my_popen3.

LD.


More information about the textmate-dev mailing list