Re: Correct ls/1 in c.erl

Bengt Kleberg <[email protected]>
Newsgroups gmane.comp.lang.erlang.patches
Organization Ericsson AB
Message-ID <[email protected]>
New name for the patch since I had a momentary lapse of reason with git.
This should include all the current suggestions.

git fetch git://github.com/ebengt/otp.git correct_ls_file_v2

https://github.com/ebengt/otp/compare/erlang:master...correct_ls_file_v2
https://github.com/ebengt/otp/compare/erlang:master...correct_ls_file_v2.patch


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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.