Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color the name with a red background if a setuid flag is set #44

Closed
Peltoche opened this issue Dec 9, 2018 · 6 comments · Fixed by #101
Closed

Color the name with a red background if a setuid flag is set #44

Peltoche opened this issue Dec 9, 2018 · 6 comments · Fixed by #101
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/enhancement Enhancement on current feature

Comments

@Peltoche
Copy link
Collaborator

Peltoche commented Dec 9, 2018

How to test:

mkdir /tmp/test

mkdir /tmp/test/dir
touch /tmp/test/file
chmod u+s /tmp/test/dir /tmp/test/file

lsd /tmp/test # `file` and `dir` should be printed with a red background
@Peltoche Peltoche added the kind/enhancement Enhancement on current feature label Dec 9, 2018
@Peltoche Peltoche added good first issue Good for newcomers help wanted Extra attention is needed and removed help wanted Extra attention is needed labels Dec 9, 2018
@loewenheim
Copy link
Contributor

My idea would be to add a boolean parameter to the colorize function to control whether the background is highlighted or not. Is this acceptable, in your opinion?

@Peltoche
Copy link
Collaborator Author

Peltoche commented Jan 16, 2019

I'm not a big fan of this kind of argument... What do you think of creating an Elem::SetuidFile and a FileType::SetuidFile like the Filetype::ExecutableFile and Elem::ExecutableFile. This will allows us to create a custom colors with a background.

What do you think of it?

@loewenheim
Copy link
Contributor

That actually makes much more sense. I initially misunderstood how the Elem enum works.

@loewenheim
Copy link
Contributor

loewenheim commented Jan 18, 2019

Another way to do it would be to make the File variants of the Elem and FileType enums parametric:

enum Elem {
[…]
File {
    uid: bool,
    exec: bool
},
[…]
}

By adding SetuidFile variants, we can replicate the behaviour of gnu ls, i.e. background red if suid, otherwise foreground green if executable, otherwise foreground white. But we cannot do a combination of foreground and background color because the file can only be a SetuidFile or an ExecutableFile.

@Peltoche
Copy link
Collaborator Author

Peltoche commented Jan 19, 2019

Seems a great idea indeed. I think you can do the same thing with the directories.

@loewenheim
Copy link
Contributor

Ok, then I’ll do it this way :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/enhancement Enhancement on current feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants