Hello Dylan,

On Sun, Dec 5, 2010 at 6:33 AM, Dylan Smith <dylan.ah.sm...@gmail.com> wrote:
> I thought a good test case for fish_indent was to try it on all the fish
> functions (i.e. share/functions/*.fish). I used the following command:
>
>> for f in *.fish; diff "$f" (../../fish_indent < "$f" | psub); end | vim -R -
> (Note: I used vim to show whitespace and for syntax highlighting)
>
> Here are my observations:
>
> In __fish_complete_ls.fish I found that "and" commands get split onto the
> next line:
> -       command ls --version >/dev/null ^/dev/null; and set is_gnu --is-gnu
> +       command ls --version > /dev/null ^ /dev/null
> +       and set is_gnu --is-gnu
> A good heuristically for fish_indent would be to keep the "and" and "or"
> commands on the same line. Perhaps this will cause long lines in some
> cases, but if the "and" command gets split onto another line then perhaps
> it should be indented.
>

Hmm, pretty good catch. If we can do, users should be able to set the longest
line length in fish_indent.(like textwidth in vim ;)

> Above you might also have noticed ^/dev/null get a space added between the
> redirection symbol and the filename. This frequently occurs, because it
> seems as if it is more common to omit the space, so should fish_indent be
> changed to omit this space?
>

Yeah. That will be good to me.

> In __fish_complete_ls.fish I found tabs get inserted before inline comments:
> -               complete $cmds -l dereference-command-line-symlink-to-dir 
> #--description "Follow directory symlinks from command line"
> +               complete $cmds -l dereference-command-line-symlink-to-dir     
>           #--description "Follow directory symlinks from command line"
> Grissiom fixed the problem with pipes, but I guess we need a more general
> solution.
>
> In my personal fish_prompt.fish function (attached):
> -               if begin; test $USER = root; and set -q fish_color_cwd_root; 
> end
> -                       set -g __fish_prompt_cwd (set_color 
> $fish_color_cwd_root)
> -               else
> -                       set -g __fish_prompt_cwd (set_color $fish_color_cwd)
> +               if begin
> +                       test $USER = root
> +                       and set -q fish_color_cwd_root
>                end
> +               set -g __fish_prompt_cwd (set_color $fish_color_cwd_root)
> +       else
> +               set -g __fish_prompt_cwd (set_color $fish_color_cwd)
>        end
> +end
>
> -       set -l error_code ""
> -       if test $cmd_status -ne 0
> -               set error_code (set_color -o red)":$cmd_status"
> -       end
> +set -l error_code ""
> +if test $cmd_status -ne 0
> +       set error_code (set_color -o red)":$cmd_status"
> +end
>
> -       set -l prompt_char '>'
> -       test $USER = root; and set prompt_char '#'
> +set -l prompt_char '>'
> +test $USER = root
> +and set prompt_char '#'
>
> -       printf (set_color green)'%s%s%s:%s%s%s%s%c ' $USER
> $__fish_prompt_hostname "$__fish_prompt_normal" "$__fish_prompt_cwd"
> (prompt_pwd) "$error_code" "$__fish_prompt_normal" "$prompt_char"
> +printf (set_color green)'%s%s%s:%s%s%s%s%c ' $USER
> $__fish_prompt_hostname "$__fish_prompt_normal" "$__fish_prompt_cwd"
> (prompt_pwd) "$error_code" "$__fish_prompt_normal" "$prompt_char"
>  end
> --
> In this case I used (if begin ...) which should result in two indentations
> added for that line. Instead a single indentation is added, so everything
> afterwards has one less indentation that it should.
>

Good catch. But might not easy to fix...

>
> If we could fix these bugs, then I think we could run fish_indent on all
> the fish functions to eat our own dog food, and to keep the coding style
> consistent for the fish code.
>
> This would also make it easier to detect regressions in fish_indent,
> because the diff from running over the same files would only
> show the results of fixes or regressions.
>
>
> P.S. I found bash was a lot faster when executing the equivalent command
> to diff all those files, but that doesn't have to do with fish_indent.
>
> $ time bash -c 'for f in *.fish; do diff "$f" <(../../fish_indent <"$f"); 
> done > /dev/null'
> real    0m0.559s
> user    0m0.688s
> sys     0m0.544s
> $ time fish -c 'for f in *.fish; diff "$f" (../../fish_indent < "$f" | psub); 
> end > /dev/null'
> real    0m3.832s
> user    0m5.004s
> sys     0m2.416s
>

Yeah, the same rate here(fish is more than 6 times slower than bash).
But if we remove
psub pipe:

...sioms-fish-shell/share/functions> time bash -c 'for f in *.fish; do
diff "$f" "$f"; done > /dev/null'
0.22user 0.08system 0:00.32elapsed 92%CPU (0avgtext+0avgdata 6672maxresident)k
0inputs+0outputs (0major+35223minor)pagefaults 0swaps

...sioms-fish-shell/share/functions>  time fish -c 'for f in *.fish;
diff "$f" "$f"; end > /dev/null'
0.60user 0.21system 0:00.84elapsed 96%CPU (0avgtext+0avgdata 6928maxresident)k
0inputs+0outputs (0major+45222minor)pagefaults 0swaps

the factor won't be that much...(I run the commands in several times
in order to let OS cache
be the same for the comparison) But the factor is still big(fish is 3
times slower).

-- 
Cheers,
Grissiom

------------------------------------------------------------------------------
What happens now with your Lotus Notes apps - do you make another costly 
upgrade, or settle for being marooned without product support? Time to move
off Lotus Notes and onto the cloud with Force.com, apps are easier to build,
use, and manage than apps on traditional platforms. Sign up for the Lotus 
Notes Migration Kit to learn more. http://p.sf.net/sfu/salesforce-d2d
_______________________________________________
Fish-users mailing list
Fish-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/fish-users

Reply via email to