On Thu, 2011-03-17 at 07:01 +-0000, Marc Weber wrote: +AD4 Hi Amit Agarwal's, +AD4 +AD4 I did a small review. +AD4 +AD4 Summary: +AD4 - do research before starting work I did. If you feel I missed something in my research, point me to it. +AD4 - Learn how to code. This means avoiding duplication. +AD4 Nice comments are fine. Working code is always better. I really did not see any code in the script that does not work. Can you be more precise. +AD4 - You can publish it nicer. Its hard to read within the small text box.. +AD4 (eg paste sites such as dpaste.com are nice for short time hosting) Thanks for the feedback. I have corrected the issue and now the site should look much better. +AD4 +AD4 Nobody should spend another minute on this script cause it contains obvious +AD4 bugs. Kindly point them. I have no issue no one spending a minute on it but would really appreciate if you could tell me the reasons. I hope you have tried to use the script before saying this and understood what the script does. +AD4 +AD4 So if you'd upload it to Vim I'd ignore or down vote it because I don't like +AD4 the style (long lines) and because of the flaws I've written down below. +AD4 (You keep trying and ask for feedback. You'll learn fast) This I completely agree. +AD4 +AD4 +AD4 I would like to put that on www.vim.org site, if that is possible. +AD4 You have to register. Then you can upload it.. +AD4 +AD4 +AD4 Please help me with your comments and reviews and if you think this will +AD4 +AD4 be helpful for the vim users. +AD4 You're too late - you should do better research. +AD4 https://github.com/MarcWeber/vim-addon-manager/blob/master/doc/vim-addon-manager.txt +AD4 Scroll down to +ACI-related work+ACI. I did not find anything similar in this link. My script can be used to list the recent additions, list the scripts with their ID number, add to GLVS (GetLatestVimScripts), show you the description given the script ID, search for text in description of plugin and so on. Can you point me to some script, which does all this. I might have missed finding the right tool and ended up re-inventing the wheel. +AD4 +AD4 Vim users do no longer need yet another solution. I encourage you to +AD4 join this (or one of the other) efforts. +AD4 +AD4 If you want others to review your code upload it somewhere else in an +AD4 indentation preserving way (oh there is a link .. found it) +AD4 +AD4 Moreover your script does not handle: +AD4 - tar +AD4 - zip +AD4 - (.vba) (vimball) +AD4 archives. Yes. That is clearly mentioned in the first line of my original post itself. +AD4 +AD4 Learn to do research - and join consider asking on irc or the +AD4 mailinglist unless its fun for you to spend your time on reinventing the +AD4 wheel :) Not all wheels are same. I don't want to put a Car wheel in Truck. If you like the wheel, its completely your choice. +AD4 +AD4 issues: +AD4 - your text version http://blog.amit-agarwal.co.in/wp-content/uploads/2011/03/get+AF8-vim+AF8-scripts.txt +AD4 is lacking the shebang +ACMAIQ-/bin/sh line so won't run at all? That because my server will not allow shebang. So, you can run with +ACI. name+ACI or +ACI-bash name+ACI. +AD4 You should be using set -e always (if something fails the script should +AD4 fail) I agree. In case of this script mostly the failures should not result in the script to terminate as the error could possibly not affect anything else. +AD4 +AD4 The +ACI-revision+ACI is the same, but the text contents are not. So your revision +AD4 system does not work. +AD4 (I was comparing the text box with the .txt file) Did you do a diff? +AD4 +AD4 - some comments don't match. NAME: get+AF8-list should be search+AF8-list .. +AD4 (This happens to me as well. That's why I only use comments without +AD4 names - and only comments if necessary ..) Something that can be corrected is better than something that has to be created, I believe. I might be wrong but like it so follow it. +AD4 +AD4 - Have you ever run this code +ACE? +AD4 +AFsAIwAh-/bin/bash+AF0: +AD4 +AD4 while true +AD4 do +AD4 +AD4 A +ADs is missing. +AD4 +AD4 - The link above contains: +AD4 +AD4 if +AFs +ACI-x+ACQ-1+ACI +AD0 +ACI-x+ACI +AF0 +AD4 then +AD4 cat +ACQ-tmp+AF8-file +AHw grep width+AHw grep script+AF8-id+AD0 +AHw-sed 's/.+ACo-script+AF8-id+AD0-//'+AHw-tr -d +ACIAXAAiACIAfA-tr +ACIAPgAi +ACI +ACIAfA-sed 's/+ADw.+ACo-//' +AHw tr -d '+AFw-001'-'+AFw-011''+AFw-013''+AFw-014''+AFw-016'-'+AFw-037''+AFw-200'-'+AFw-377' +AHw-grep -i +ACIAJA-text+ACIAfAAk-PAGER +AD4 else +AD4 cat +ACQ-tmp+AF8-file +AHw grep width+AHw grep script+AF8-id+AD0 +AHw-sed 's/.+ACo-script+AF8-id+AD0-//'+AHw-tr -d +ACIAXAAiACIAfA-tr +ACIAPgAi +ACI +ACIAfA-sed 's/+ADw.+ACo-//' +AHw tr -d '+AFw-001'-'+AFw-011''+AFw-013''+AFw-014''+AFw-016'-'+AFw-037''+AFw-200'-'+AFw-377' +AHw-grep -i +ACIAJA-text+ACIAfAAk-PAGER +AD4 fi +AD4 +AD4 Do you want to know how much time I've spend on comparing char by char to +AD4 find out that both are the same lines? I think it should not take more than couple of seconds to figure that out. That was a redundant code and had left it by mistake. Appreciate the feedback on this. +AD4 +AD4 Do something like this instead: +AD4 +AD4 my+AF8-filter()+AHs +AD4 cat +AFw +AD4 +AHw sed .. +AFw +AD4 +AHw grep .. +AFw +AD4 +AH0 +AD4 +AD4 if .. +AD4 then +AD4 input +AHw filter +AHw output +AD4 else +AD4 input +AHw filter +AHw output +AD4 fi Yes, this would definately be better. +AD4 +AD4 Its much more readable+ACE +AD4 +AD4 That's enough to make me move your script to /dev/null. +AD4 +AD4 Marc Weber +AD4
-- Thanks, -aka http://blog.amit-agarwal.co.in http://shop.amit-agarwal.co.in http://amit-agarwal.com -- 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
