Hi Amit Agarwal's,

I did a small review.

Summary: 
- do research before starting work
- Learn how to code. This means avoiding duplication.
  Nice comments are fine. Working code is always better.
- You can publish it nicer. Its hard to read within the small text box..
  (eg paste sites such as dpaste.com are nice for short time hosting)

Nobody should spend another minute on this script cause it contains obvious
bugs.

So if you'd upload it to Vim I'd ignore or down vote it because I don't like
the style (long lines) and because of the flaws I've written down below.
(You keep trying and ask for feedback. You'll learn fast)

> I would like to put that on www.vim.org site, if that is possible.
You have to register. Then you can upload it..

> Please help me with your comments and reviews and if you think this will
> be helpful for the vim users.
You're too late - you should do better research.
https://github.com/MarcWeber/vim-addon-manager/blob/master/doc/vim-addon-manager.txt
Scroll down to "related work".

Vim users do no longer need yet another solution. I encourage you to
join this (or one of the other) efforts.

If you want others to review your code upload it somewhere else in an
indentation preserving way (oh there is a link .. found it)

Moreover your script does not handle:
  - tar
  - zip
  - (.vba) (vimball)
archives.

Learn to do research - and join consider asking on irc or the
mailinglist unless its fun for you to spend your time on reinventing the
wheel :)

issues:
- your text version 
http://blog.amit-agarwal.co.in/wp-content/uploads/2011/03/get_vim_scripts.txt
 is lacking the shebang #!/bin/sh line so won't run at all?
 You should be using set -e always (if something fails the script should
 fail)

  The "revision" is the same, but the text contents are not. So your revision
  system does not work.
  (I was comparing the text box with the .txt file)

- some comments don't match. NAME: get_list should be search_list ..
  (This happens to me as well. That's why I only use comments without
  names - and only comments if necessary ..)

-  Have you ever run this code !?
  [#!/bin/bash]:

    while true
    do

  A ; is missing.

- The link above contains:

  if [ "x$1" = "x" ]
  then
       cat $tmp_file | grep width| grep script_id= |sed 's/.*script_id=//'|tr 
-d "\""|tr ">" " "|sed 's/<.*//' | tr -d 
'\001'-'\011''\013''\014''\016'-'\037''\200'-'\377' |grep -i "$text"|$PAGER
  else
       cat $tmp_file | grep width| grep script_id= |sed 's/.*script_id=//'|tr 
-d "\""|tr ">" " "|sed 's/<.*//' | tr -d 
'\001'-'\011''\013''\014''\016'-'\037''\200'-'\377' |grep -i "$text"|$PAGER
  fi

  Do you want to know how much time I've spend on comparing char by char to
  find out that both are the same lines?

  Do something like this instead:

  my_filter(){
    cat \
    | sed .. \
    | grep .. \
  }

  if ..
  then
    input | filter | output
  else
    input | filter | output
  fi

  Its much more readable!

  That's enough to make me move your script to /dev/null.

Marc Weber

-- 
You received this message from the "vim_use" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

Reply via email to