On Wed, Nov 14, 2012 at 11:53:54PM +0900, Kazuhiko Sakaguchi wrote:
> Thanks! I fixed problems.
> Please see https://github.com/pi8027/pane-maximize/blob/master/pane-maximize .

It would be handier to see in-line diffs to be honest, rather than forcing
me to load a web browser each time.

Here it is though:

> diff --git a/pane-maximize b/pane-maximize
> index f38d7ab..4dab20e 100755
> --- a/pane-maximize
> +++ b/pane-maximize
> @@ -1,43 +1,45 @@
> -#!/bin/bash
> +#!/usr/bin/env bash

I'm starting to wonder if "bash" is good enough here.  You do use some
bashisms (which I'll get to in a moment) but I think being as portable as
possible (since I know we have AIX and SunOS users, etc., of tmux) might be
worthwhile.  That is to say, assume sh(1).

>  # By Kazuhiko Sakaguchi. Public domain.
>
>  function usage_exit(){
> -cat <<EOT
> -Usage: $(echo $0 | sed "s/.*\///") [-adP] [-F format] [-t target-pane]
> +cat <<EOT >&2
> +Usage: $(basename $0) [-adP] [-F format] [-t target-pane]
>  -a, -d, -P, -F format:   these options are passed to tmux new-window
>  -t target-pane:          specify target pane

Why aren't these options just passed straight through to tmux?  That is,
your script can just shunt "$@" through to tmux directly.  I see no reason
or need for these checks to be part of your script.  Not to mention it has
the added advantage of letting tmux do the error handling with newer
options, etc., where your script might not be up to date with those if any
of those commands ever learned any new options for example.

> +    $([[ -n $paneid ]] && echo "-t $paneid") -p '#{pane_id}')

There's many examples of you suddenly introducing "[[" here which is the
so-called "new test" operator over the standard "[" (which is typically
these days a shell-builtin, but otherwise a symlink to /bin/test).  Why have
you done this?  None of the tests you're performing inside [[..]] make use
of the features, so you can just go back to using [..] blocks, and be kinder
to older shells.

> -    tmux display-message "This pane is already maximized."
> +    tmux display-message 'This pane is already maximized.' >&2

Why the shunt to stderr here?  You're not using -p (which goes to STDOUT
anyway).

More fundamentally though your script is still broken for me.  Just
splitting a window in two and running:

.../pane-maximize/pane-maximize -a

results in something trying to happen, but certainly my pane isn't zoomed at
all.  In fact, the whole script ends up trying to do this:

tmux new-window -a -t @5
'"/home/n6tadam/projects/pane-maximize/pane-maximize-sub" %37'

Which is failing for some reason.

I'll leave you to look in to this, since I've got other things I need to do.

-- Thomas Adam

------------------------------------------------------------------------------
Monitor your physical, virtual and cloud infrastructure from a single
web console. Get in-depth insight into apps, servers, databases, vmware,
SAP, cloud infrastructure, etc. Download 30-day Free Trial.
Pricing starts from $795 for 25 servers or applications!
http://p.sf.net/sfu/zoho_dev2dev_nov
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users

Reply via email to