Re: Correct ls/1 in c.erl

Greetings,

After I updated c.erl and c_SUITE.erl according to your suggestions git
was unhappy. It hinted "git rm c.erl" would help. So I tried that. Now 
https://github.com/ebengt/otp/compare/master...correct_ls_file
is really big ("This comparison is big! We're only showing the most
recent 250 commits").
Unless there is a simple way to solve this I will start from scratch and
get back to you.


bengt

On Tue, 2013-05-14 at 10:46 +0200, Björn Gustavsson wrote:
> On Mon, May 6, 2013 at 7:01 AM, Bengt Kleberg
> <[email protected]> wrote:
>         Greetings,
>         
>                 When I use c:ls/1 it reminds me so much of Unix "ls"
>         that I
>         expect c:ls("filename") to work. The resulting error surprises
>         me every
>         time (not the same day). Here is a patch that corrects c:ls/1
>         to be more
>         like Unix "ls".
>         
> 
> 
> Thanks for your patch.
> 
> 
> The original code was written a long time ago, before the
> functions in the file module returned documented error
> reasons and before file:format_error/1 was introduced.
> 
> 
> Therefore, if the code is to be updated, it should be
> updated properly, by matching for enotdir and generating
> the error messages for other errors using file:format_error/1.
> 
> 
> Also, a test case is needed to ensure that the code works
> and that it continues to work.
> 
> 
> 
> 
> Your current version of the patch has two issues.
> 
> 
> First, there is the style issue. Quoting
> https://github.com/erlang/otp/wiki/Submitting-patches:
> 
> 
>   "Make sure your changes follow the coding and indentation style of
> the code surrounding your changes."
> 
> 
> (There should not be any extra spaces inside parenthesis.)
> 
> 
> Second, filelib:is_file/1 returns true for both files and directories.
> You probably want to use filelib:is_regular/1.
> 
> 
> /Bjorn
>  
> -- 
> Björn Gustavsson, Erlang/OTP, Ericsson AB

_______________________________________________
erlang-patches mailing list
[email protected]
http://erlang.org/mailman/listinfo/erlang-patches