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