! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
xfce4-settings-manager cleanup
Status:
RESOLVED: WONTFIX
Product:
Xfce4-settings
Component:
Settings Manager

Comments

Description Nick Schermer editbugs 2008-06-29 16:43:06 CEST
Patch cleans the xfce4-settings-manager code.
* Use XfceRc for desktop files (less string dups and is cleaner).
* Use cell renders.
* Various cleanups.
Comment 1 Nick Schermer editbugs 2008-06-29 16:44:14 CEST
Created attachment 1716 
Patch
Comment 2 Brian J. Tarricone (not reading bugmail) 2008-06-29 17:25:01 CEST
NACK on the use of XfceRc instead of XfceDesktopEntry.  I don't care about the allocations; it's there for convenience and code clarity, not performance (like most abstractions).

Stop changing my indent style, spacing, and trivial stuff.  I want to see just what you're actually changing, and I don't accept patches that don't use my coding style anyway.

If you want to just submit a patch that uses cell renderers and doesn't change *anything* else, that would be great.
Comment 3 Nick Schermer editbugs 2008-06-29 18:33:18 CEST
Created attachment 1717 
Another patch

Well the use of XfceRc is more clear if you'd ask me, so i'm not goign to change that. There is nothing more convienent by using xfcedesktop entry and i bet you didn't even looked at the resulting code.

Other changes in the patch compared to the previous one:
* Disable selection for now (including search) since the text renderer does not support selection (white label after a click is confusing).
* Changed the indentation and bracket style...
* Add basic goption entries.
Comment 4 Brian J. Tarricone (not reading bugmail) 2008-06-30 03:30:59 CEST
So then how do you easily read the localised names from the .desktop files using XfceRc without making it really complicated and duplicating code?  Your patch will always use the English string for the icon label, so it's no good.
Comment 5 Nick Schermer editbugs 2008-06-30 06:37:08 CEST
That's not true, xfce_rc_read_entry will always try to read the translated string, xfce_rc_read_entry_untranslated will always read the english string.

Infact, I should have use xfce_rc_read_entry_untranslated for the Exec and Icon entries, but since they are both not marked as translatable it's not really an issue.

(Btw. Thunar also uses XfceRc for reading desktop files, while Benny could have use XfceDesktopEntry)
Comment 6 Brian J. Tarricone (not reading bugmail) 2008-06-30 17:54:29 CEST
(In reply to comment #5)
> That's not true, xfce_rc_read_entry will always try to read the translated
> string, xfce_rc_read_entry_untranslated will always read the english string.

Ah, good point, I didn't think of that.

> (Btw. Thunar also uses XfceRc for reading desktop files, while Benny could have
> use XfceDesktopEntry)

Yes, well, Benny tends to like to use code he's written over code written by others.  Often it's simply because his code is better, sometimes it's just because he's Benny, and that's what he does.

Your change is by no means required, and obscures the actual useful changes (like using GtkCellLayout).  You've removed the list store sort function without an explanation, and there's a label 'close_rc' that isn't explained at all (what kind of useless comment is "/* label */"?  of course it's a label!).  You've still changed my indentation style, which is annoying, and you've done useless stuff like rename the variable "snotify" to "startup_notity" -- and misspelled it in the process!  I imagine there are other annoyances, but I really don't feel like spending the time to try to decipher the rest of your patch.  If you have a change to make, just make the change (and nothing else!), and avoid all this other cosmetic crap that I don't like and don't want.

I appreciate you offering to do some work, but this isn't the time to "make the code yours."  Bottom line: this is my code, and I have to maintain it, not you.  My time is limited, and I'd rather not spend it accepting code changes that I'll have to think harder to understand when I look at it in 6 months.
Comment 7 Nick Schermer editbugs 2008-06-30 18:16:11 CEST
Done arguing. 

Bug #4185

Reported by:
Nick Schermer
Reported on: 2008-06-29
Last modified on: 2009-07-14

People

Assignee:
Brian J. Tarricone (not reading bugmail)
CC List:
3 users

Version

Attachments

Patch (20.25 KB, patch)
2008-06-29 16:44 CEST , Nick Schermer
no flags
Another patch (22.52 KB, patch)
2008-06-29 18:33 CEST , Nick Schermer
no flags

Additional information