Page MenuHomeFreeBSD

ls: Support dired mode
Needs ReviewPublic

Authored by aokblast on Fri, May 29, 8:40 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 11, 1:32 PM
Unknown Object (File)
Wed, Jun 10, 10:40 PM
Unknown Object (File)
Wed, Jun 10, 1:16 AM
Unknown Object (File)
Wed, Jun 10, 1:10 AM
Unknown Object (File)
Mon, Jun 8, 11:06 PM
Unknown Object (File)
Mon, Jun 8, 10:12 PM
Unknown Object (File)
Mon, Jun 8, 7:34 PM
Unknown Object (File)
Sun, Jun 7, 4:21 PM
Subscribers

Details

Reviewers
imp
jrm
Summary

Emacs continuously complains that dired mode is not supported here. As a
result, we implemented dired mode support. When this option is enabled,
a //DIRED// note is generated to indicate the byte offsets marking the
beginning and end of each filename.

We do not implement //DIRED-OPTIONS//. Note that --dired is commonly
abbreviated as -D, but this conflicts with an existing flag, so we do
not provide that abbreviation.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 73696
Build 70579: arc lint + arc unit

Event Timeline

Very cool.

The docstring for the dired-use-ls-dired variable provides a bit more context.

Note that if you set this option to nil, either through choice or
because your "ls" program does not support "--dired", Dired
will fail to parse some "unusual" file names, e.g. those with leading
spaces.  You might want to install ls from GNU Coreutils, which does
support this option.

Assuming there are no objections, we'll also need to update ls(1).

I only have a few moments now, so I'll just say I support this. On quick inspection, I just see the issue with the missing break;. I'll come back to look more closely when I have more time.

bin/ls/ls.c
490

I think you now need a break; after this line.

In D57322#1313478, @jrm wrote:

Very cool.

The docstring for the dired-use-ls-dired variable provides a bit more context.

Note that if you set this option to nil, either through choice or
because your "ls" program does not support "--dired", Dired
will fail to parse some "unusual" file names, e.g. those with leading
spaces.  You might want to install ls from GNU Coreutils, which does
support this option.

Assuming there are no objections, we'll also need to update ls(1).

I only have a few moments now, so I'll just say I support this. On quick inspection, I just see the issue with the missing break;. I'll come back to look more closely when I have more time.

I don't see any difference from the origianl and current version of ls on emacs side. But it makes emacs stops complaining:).

I believe Emacs calls ls --dired and pipes the output. I think that means we have a problem because you are using ftell(stdout) (*in three different places), which will always return -1 when the output is redirected to a pipe.

jrm@asn /bin % ls --dired /bin | cat
total 1360
-r-xr-xr-x  2 root wheel  11384 May 24 15:00 [
-r-xr-xr-x  1 root wheel  13424 May 24 15:00 cat
...
-r-xr-xr-x  2 root wheel  12584 May 24 15:00 unlink
-r-xr-xr-x  1 root wheel   9496 May 24 15:00 uuidgen
//DIRED// 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
jrm@asn /bin % ls --dired /bin > /tmp/dired_out.txt && cat /tmp/dired_out.txt
total 1360
-r-xr-xr-x  2 root wheel  11384 May 24 15:00 [
-r-xr-xr-x  1 root wheel  13424 May 24 15:00 cat
...
-r-xr-xr-x  2 root wheel  12584 May 24 15:00 unlink
-r-xr-xr-x  1 root wheel   9496 May 24 15:00 uuidgen
//DIRED// 56 57 103 106 152 159 205 209 255 260 306 308 354 360 406 409 455 459 505 507 553 555 601 611 657 661 707 709 755 759 805 820 866 873 919 927 973 977 1023 1027 1073 1077 1123 1125 1171 1173 1219 1224 1270 1272 1318 1323 1369 1372 1418 1423 1469 1474 1520 1522 1568 1573 1619 1622 1668 1676 1722 1725 1771 1773 1819 1824 1870 1875 1921 1928 1974 1976 2022 2027 2073 2077 2123 2127 2173 2177 2223 2227 2273 2280 2326 2332 2378 2385

If we want this to work (and not just suppress the warning from Emacs), we will have to use something other than ftell().

bin/ls/ls.c
282

You added N but no case to handle it, so ls -N will always show usage information.

bin/ls/print.c
284

I don't think it matters, but aren't the arguments swapped? jemalloc(3) has void *calloc(size_t number, size_t size);.

Maybe it could be written more defensively with:

offsets = calloc((size_t)dp->entries * 2, sizeof(*offsets));
if (offsets == NULL && dp->entries != 0)
    err(1, NULL);
359

I see below in printsize(), we are using %jd to print an off_t. Can you check which is appropriate?

Accumualate bytes instead of using ftell
Also, reflects jrm@'s comment