This patch adds octal format for permissions to the KPropertiesDialog
Details
Diff Detail
- Repository
- R241 KIO
- Branch
- octal
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 11101 Build 11119: arc lint + arc unit
Parsing the result of KFileItem::permissionsString() is a rather bad idea, especially considering that KFileItem already provides mode() and permissions().
Also, the placing of the label is completely off.
I had used permission() to get the mode_t variable, which I later type casted to qint64, still the permissions resulted were completed wrong. Any suggestions, how this may be done better in efficient manner?
Tried to print it as octal-based value?
Any suggestions, how this may be done better in efficient manner?
src/widgets/kpropertiesdialog.cpp | ||
---|---|---|
1869 | You don't need to create a whole new grid layout; just add the label to the existing one. |
Also: what is the use case of this feature? For casual users the octal permissions make no sense, they are mostly useful for power users (if they don't even just use the terminal for these things).
At most, this would fit in the "advanced permissions" dialog.
I think it makes sense to show octal permissions somewhere in the GUI. Power users use the GUI, too. :) But yeah, where exactly to put it needs some thought.
We could also make the permissions interactive and explaining at the same time.
I could imagine something like this:
- Use mode_t to get the permissions
- Position the octal permissions inside "Advanced Permissions" tab
Code looks sane now. But the UI needs polish. Putting it in the advanced page feels awkward to me, and even on that page, its location isn't right. It feels arbitrary, disconnected from the rest of the UI. I feel like it might sit better on the main page, to the left of the Advanced Permissions button. Alternatively, if it has to stay on the advanced page, it needs to feel better integrated with the rest of the layout.
Adding VDG for more commentary.
Also, there was feedback it was still not taken care.
Please do not ping on pathed when you are requested for changes, and still do not do them.
src/widgets/kpropertiesdialog.cpp | ||
---|---|---|
2136 | why the cast to qint64? | |
2137 | why this string includes the untranslated label? | |
2140 | the translatable label must be here instead |
The permission changes to ktelnetservice5.desktop are unrelated, please revert them.
Mostly important, the value shown in the advanced permissions dialog is not taking into account the changes done in the properties dialog of the file. The other widgets in this advanced dialog already do, so the shown octal value ought to as well.
src/widgets/kpropertiesdialog.cpp | ||
---|---|---|
2139 | just use the static QString::number? |