Re: Correct ls/1 in c.erl

On Tue, May 14, 2013 at 3:53 PM, Bengt Kleberg
<[email protected]>wrote:

> 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
>
>
Thanks!

Your new version looks better, but I still see room for improvement.

If I try to list a non-existing file I get:

1> ls("non-existing").
no such file or directoryok
2>

It does not look pretty. There should be a new line
after the error message.

Also, attempting to list a non-existing file is not
tested in the test suite. I no that you cannot test
for the exact error message (it may be differently
worded on different platforms), but you can at least
make a smoke test so that the code will be covered.

Next issue:

2> ls("/dev/null").
Invalid directory or file
ok
3>

I find the error message confusing. What are
you trying to tell the user?

Why not simply print the file if it is not a
directory?

/Bjorn

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

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