[patch] js.patch

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] js.patch

Cornelius(c9s)
Hi, 

I have a patch for  javascript binding to vim, this is my first time sending a patch for vim.
I don't know if this work is acceptable.  i would appreciate if this could be accept.  :-)

I will continue on developing this feature.


this patch is diffed from:

Repository UUID: 2a77ed30-b011-0410-a7ad-c7884a0aa172
Revision: 1682


--
Best Regards.

Cornelius ( Yo-An Lin )
E-mail: [hidden email]

--
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php

js.patch (17K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [patch] js.patch

Lech Lorens
On 27-Nov-2009 Cornelius <[hidden email]> wrote:

> Hi,
>
> I have a patch for  javascript binding to vim, this is my first time sending
> a patch for vim.
> I don't know if this work is acceptable.  i would appreciate if this could
> be accept.  :-)
>
> I will continue on developing this feature.
>
>
> this patch is diffed from:
>
> URL: https://vim.svn.sourceforge.net/svnroot/vim/vim7
> Repository Root: https://vim.svn.sourceforge.net/svnroot/vim
> Repository UUID: 2a77ed30-b011-0410-a7ad-c7884a0aa172
> Revision: 1682
>
>
> --
> Best Regards.
>
> Cornelius ( Yo-An Lin )
> E-mail: [hidden email]


Thank you for your interest in improving Vim!

If you are unsure whether your code is acceptable, you might want to
have a look at develop.txt (:help develop.txt), especially at notes
regarding the coding style (:help coding-style). You will notice that
you have broken some of the rules stated there:
- you use the ANSI style function declarations,
- you do not place opening braces on a new line as is done throughout
  Vim source code,
- you do not declare variables at the beginning of code blocks,
- you do not put space between an 'if' and opening parenthesis,
- in 'if' statements you put the condition and the action in one line,
- you use strchr() instead of vim_strchr().

Comments from myself:
- you seem to be slightly inconsistent about whitespace: in feature.h
  you use spaces while the code around your change uses tabs,
- you left trailing spaces in a few places, while I can only find 2 such
  places in Vim so far,
- you left some commented-out code (js_rand, js_srand), which in my
  experience eventually tends to make maintaining the code harder,
- your code seems to be written with the following settings:
  sw=4 et
  while Vim's code seems to follow the standard:
  ts=8 sw=4 noet
- if FEAT_JS is not defined, you define ex_js to be ex_script_ni.
  Otherwise, there is no such thing as ex_js.

Additionally, in reportError() you use uninitialised error_msg pointer.

I have no idea about JavaScript and therefore feel incompetent to state
anything more useful about your code. Maybe others will be interested.

Keep up the good work!

--
Cheers,
Lech

--
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: [patch] js.patch

Cornelius(c9s)
Hi Lech Lorens,

  Thank you for spending time correcting me this. I really need this.
  I will correct those things in next patch. 
  Thank you.  :-)


On Sat, Nov 28, 2009 at 9:13 AM, Lech Lorens <[hidden email]> wrote:
On 27-Nov-2009 Cornelius <[hidden email]> wrote:
> Hi,
>
> I have a patch for  javascript binding to vim, this is my first time sending
> a patch for vim.
> I don't know if this work is acceptable.  i would appreciate if this could
> be accept.  :-)
>
> I will continue on developing this feature.
>
>
> this patch is diffed from:
>
> URL: https://vim.svn.sourceforge.net/svnroot/vim/vim7
> Repository Root: https://vim.svn.sourceforge.net/svnroot/vim
> Repository UUID: 2a77ed30-b011-0410-a7ad-c7884a0aa172
> Revision: 1682
>
>
> --
> Best Regards.
>
> Cornelius ( Yo-An Lin )
> E-mail: [hidden email]


Thank you for your interest in improving Vim!

If you are unsure whether your code is acceptable, you might want to
have a look at develop.txt (:help develop.txt), especially at notes
regarding the coding style (:help coding-style). You will notice that
you have broken some of the rules stated there:
- you use the ANSI style function declarations,
- you do not place opening braces on a new line as is done throughout
 Vim source code,
- you do not declare variables at the beginning of code blocks,
- you do not put space between an 'if' and opening parenthesis,
- in 'if' statements you put the condition and the action in one line,
- you use strchr() instead of vim_strchr().

Comments from myself:
- you seem to be slightly inconsistent about whitespace: in feature.h
 you use spaces while the code around your change uses tabs,
- you left trailing spaces in a few places, while I can only find 2 such
 places in Vim so far,
- you left some commented-out code (js_rand, js_srand), which in my
 experience eventually tends to make maintaining the code harder,
- your code seems to be written with the following settings:
 sw=4 et
 while Vim's code seems to follow the standard:
 ts=8 sw=4 noet
- if FEAT_JS is not defined, you define ex_js to be ex_script_ni.
 Otherwise, there is no such thing as ex_js.

Additionally, in reportError() you use uninitialised error_msg pointer.

I have no idea about JavaScript and therefore feel incompetent to state
anything more useful about your code. Maybe others will be interested.

Keep up the good work!

--
Cheers,
Lech



--
Best Regards.

Cornelius ( Yo-An Lin )
E-mail: [hidden email]
http://c9s.blogspot.com/

--
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
Reply | Threaded
Open this post in threaded view
|

Re: [patch] js.patch

Cornelius(c9s)
In reply to this post by Lech Lorens
Hi,

On Sat, Nov 28, 2009 at 9:13 AM, Lech Lorens <[hidden email]> wrote:
On 27-Nov-2009 Cornelius <[hidden email]> wrote:

Thank you for your interest in improving Vim!

If you are unsure whether your code is acceptable, you might wa
- you do not put space between an 'if' and opening parenthesis,
- in 'if' statements you put the condition and the action in one line,
- you use strchr() instead of vim_strchr().

it just remind me that the strchr() code which is copied from if_ruby.c.

--
Best Regards.

Cornelius ( Yo-An Lin )
E-mail: [hidden email]


--
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php