show @ if extended attributes are present

[accidentaldevelopment]

ls -l on macOS will show an @ if the file or directory has extended attributes. This PR adds the same functionality to lsd.

macOS also includes a -@ flag to display the extended attributes. This PR does not include that functionality. If it's something that's wanted, it should probably be done in a followup PR.

Example for this repo

❯ cargo r -- -l
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/lsd -l`
.rw-r--r--@ user group 1.7 KB Thu Nov  9 16:27:32 2023  build.rs
.rw-r--r--@ user group  39 KB Thu Nov  9 16:27:32 2023  Cargo.lock
   ...omitted
drwxr-xr-x@ user group 192 B  Thu Nov  9 16:32:28 2023  target
drwxr-xr-x@ user group  96 B  Thu Nov  9 16:27:32 2023 󰙨 tests

Concerns

This adds a new has_xattr field to the AccessControl struct. That field is determined by checking selinux_context, smack_context, and just listing xattrs.

If my understanding and testing are correct, this should never really show up in anything other than macOS. But the code and field will be present anyway. It feels a little dirty having a field that will never really be used outside of a single platform. But the alternative is some conditional compilation, and I generally feel that should be approached with care.

Happy to discuss alternative implementations!

---

TODO

• Use cargo fmt
• Add necessary tests
• Update default config/theme in README (if applicable)
• Update man page at lsd/doc/lsd.md (if applicable)

[muniu-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: accidentaldevelopment

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[zwpaper]

hi @accidentaldevelopment, may I ask whether this is the macOS-only feature, not the GNU ls?

if so, we should leave it macOS-only.

if there will later be some linux requirements, we could put it behind a flag, and should not enable by default in linux

[accidentaldevelopment]

Hello, @zwpaper, thanks for the reply!

The answer to your question is exactly why this one could be a little ugly. Linux currently checks for specific xattrs (system.posix_acl_access, security.selinux) to determine if it should show . or +. However macOS will show @ if there are any xattrs on the file.

ls on Linux, to my knowledge, doesn't give any particular indication for arbitrary xattrs, just the few known ones that lsd already shows. While macOS seems to just have the single catchall for any xattrs.

Related is a macOS-specific flag -@ to list those xattrs. That functionality is not part of this PR.

Does that help? It's kind of a weird divergence between the two OS families.

[zwpaper]

hi @accidentaldevelopment, thanks for the explanation, the problem is that it will break the linux users if the @ appear by default especially when we were trying to align with the GNU ls.

so I would prefer showing @ only on macOS, so we will not break any linux use-cases.

[CarterLi]

Maybe adding an option to control it is a better idea?

[accidentaldevelopment]

Whoops! Sorry, @zwpaper – I missed the notification for the replies!

so I would prefer showing @ only on macOS, so we will not break any linux use-cases.

Makes sense. There are a few different ways I could think of to do this:

1. Hide the entire has_xattr field behind a cfg so it only exists on macOS. This may be the most correct indication on a given OS, but it would also make for several extra cfg blocks to detect and print.
2. In render_method, just ignore has_xattr if we're not on macOS. Little awkward in the if statement, but otherwise pretty straight-forward.
3. In AccessControl::from_path, always set has_xattr to false unless we're on macOS. Probably the simplest since all other logic is the same - it's just always false on linux. Avoids specialized tests as well. But it's an awkward implementation detail existing in a function. If any other constructor functions are added, they'd have to follow the same rules.

I'm open to any of the above, or other options I may have missed. Obviously I would want to do whatever is best for the codebase and experience!

Maybe adding an option to control it is a better idea?

I'm a little unsure what you mean here, @CarterLi. Are you suggesting it goes behind a command line flag? This would break compatibility with ls on macOS, and be rather unintuitive I think.

Or are you referring to a config file option? That could probably work quite well, and default to true on macOS.