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

storaged: btrfs: show btrfs filesystem "root" subvolume as "top-level" #21127

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jelly
Copy link
Member

@jelly jelly commented Oct 16, 2024

Showing this as "/" confuses users as they might think it is the root partition and official btrfs terminology names it "top-level".

https://btrfs.readthedocs.io/en/latest/Subvolumes.html

Relates: #19920


image

@jelly jelly added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Oct 16, 2024
@jelly jelly requested a review from garrett October 16, 2024 15:02
@jelly
Copy link
Member Author

jelly commented Oct 16, 2024

If we want this italic, it might not be too easy and we have to special case it for btrfs.

@garrett
Copy link
Member

garrett commented Oct 17, 2024

Two different naive approaches, without much code modification:

  • One passes all the names to a data attribute and CSS specifically acts upon the attribute (with a simple class selector child below it, so it won't try to match anything but a parent of the class, instead of everything): b33a512

  • The other explicitly only handles "top-level" and sets a class: 3a660b1

This is assuming that "name" is not translated here, however, and that subvolumes are not able to be called "top-level" manually, and that someone has done so.

Is this the best way to do it? I'm sure it's not. It's a way without modifying anything. It might spark some other idea too (perhaps a better way to determine this, as you know the code better), so I'm sharing it.

garrett
garrett previously approved these changes Oct 17, 2024
Copy link
Member

@garrett garrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Also, I'm fine if we merge this as-is and treat styling as a follow-up. This is already an improvement.

However: One quick question: Is top-level ever supposed to be translated? It's the btrfs internal name for it, right? Do btrfs docs translate it? Do the command line tools translate it?

@garrett
Copy link
Member

garrett commented Oct 17, 2024

It's still in English as "top-level", at least in https://wiki.debianforum.de/Btrfs :

image

Showing this as "/" confuses users as they might think it is the root
partition and official btrfs terminology names it "top-level".

https://btrfs.readthedocs.io/en/latest/Subvolumes.html

Relates: cockpit-project#19920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-test For doc/workflow changes, or experiments which don't need a full CI run,
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants