Re: Review Request: The KSystemActivityDialog uses MB as default unit.

2010-11-30 Thread John Tapsell
On 30 November 2010 16:08, Matthias Fuchs ma...@gmx.net wrote:
 how it is displayed to the user:
 1) 56 K
 2) 3.7 M
 3) 5 G

It takes a lot more effort to see that the last item is using more
memory.  Especially if you're not that comfortable with the K, B, G
meanings.

The 56 K  takes up more room and looks bigger than 5 G despite
being much much smaller.

 Alternatively adding seperators to make the numbers more readable could be an
 option:
 firefox  matthias  3% 650,292 K
 vs as it is now
 firefox  matthias  3% 650292 K

This could be done, but increases the size of the column a tiny bit.
Maybe worth the sacrifice though.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: The KSystemActivityDialog uses MB as default unit.

2010-11-28 Thread John Tapsell

---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6007/#review9029
---


Sorry, but this is too much of a hack, and I don't really agree with the idea 
of the change anyway.

If you do a quick poll somewhere and show that other people agree it should be 
MB by default, then I'll change it.  But I don't want to change it just based 
on 1 person.

- John


On 2010-11-28 23:31:11, Matthias Fuchs wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6007/
 ---
 
 (Updated 2010-11-28 23:31:11)
 
 
 Review request for Plasma, Aaron Seigo and John Tapsell.
 
 
 Summary
 ---
 
 The KSystemActivityDialog uses MB as default unit.
 BUG:222022
 
 
 This addresses bug 222022.
 https://bugs.kde.org/show_bug.cgi?id=222022
 
 
 Diffs
 -
 
   /trunk/KDE/kdebase/workspace/krunner/ksystemactivitydialog.cpp 1201754 
 
 Diff: http://svn.reviewboard.kde.org/r/6007/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Matthias
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: The KSystemActivityDialog uses MB as default unit.

2010-11-28 Thread John Tapsell


 On 2010-11-28 23:39:49, Aaron Seigo wrote:
  i think the real problem is in KSysGuardProcessList::loadSettings(const 
  KConfigGroup cg) where it uses hard coded defaults such as this line:
  
   setUnits((ProcessModel::Units) cg.readEntry(units, 
  (int)ProcessModel::UnitsKB));
  
  it should be possible to do:
  
  m_processList.setUnits(ProcessModel::UnitsMB);
  m_processList.loadSettings(config);
  
  and have the current settings used as the defaults. the hardcoded defaults 
  can be set up in the ctor of KSysGuardProcessList. so the setUnits line 
  above would become:
  
  setUnits((ProcessModel::Units) cg.readEntry(units, 
  (int)d-m_model.units()));
  
  thoughts?

Yep, makes a lot of sense.


- John


---
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6007/#review9030
---


On 2010-11-28 23:31:11, Matthias Fuchs wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://svn.reviewboard.kde.org/r/6007/
 ---
 
 (Updated 2010-11-28 23:31:11)
 
 
 Review request for Plasma, Aaron Seigo and John Tapsell.
 
 
 Summary
 ---
 
 The KSystemActivityDialog uses MB as default unit.
 BUG:222022
 
 
 This addresses bug 222022.
 https://bugs.kde.org/show_bug.cgi?id=222022
 
 
 Diffs
 -
 
   /trunk/KDE/kdebase/workspace/krunner/ksystemactivitydialog.cpp 1201754 
 
 Diff: http://svn.reviewboard.kde.org/r/6007/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Matthias
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Show shortcut for System Activity in krunner

2009-12-01 Thread John Tapsell

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2308/
---

(Updated 2009-12-01 12:12:29.713593)


Review request for Plasma.


Summary (updated)
---

This shows the global shortcut in the tooltip for the System Activity button in 
the default krunner interface.

It required rearranging the code slightly in krunnerapp so that the interface 
is created after the shortcuts are created.

NOTE:

This adds the string i18n(%1 (%2))but we are currently in string freeze...


Diffs
-

  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1056420 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1056420 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1056420 

Diff: http://reviewboard.kde.org/r/2308/diff


Testing
---

Compiled, ran.  Hovered over and saw that the tooltip had changed.  Then went 
to system settings and changed the shortcut, and saw that the tooltip updated 
instantly.


Thanks,

John

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request: Show shortcut for System Activity in krunner

2009-11-30 Thread John Tapsell

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2308/
---

Review request for Plasma.


Summary
---

This shows the global shortcut in the tooltip for the System Activity button in 
the default krunner interface.

It required rearranging the code slightly in krunnerapp so that the interface 
is created after the shortcuts are created.


Diffs
-

  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.h 1056420 
  trunk/KDE/kdebase/workspace/krunner/interfaces/default/interface.cpp 1056420 
  trunk/KDE/kdebase/workspace/krunner/krunnerapp.cpp 1056420 

Diff: http://reviewboard.kde.org/r/2308/diff


Testing
---

Compiled, ran.  Hovered over and saw that the tooltip had changed.  Then went 
to system settings and changed the shortcut, and saw that the tooltip updated 
instantly.


Thanks,

John

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Adding a string

2009-11-30 Thread John Tapsell
Hi all,

  I have a review request for to show shortcut for System Activity in
krunner, but it adds a new string  %1 (%2).  Can I get an exception,
or shall I make it non-translated, or delay the commit?

John
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Bugs in signalplotter.cpp

2009-09-22 Thread John Tapsell
2009/9/21 Aaron J. Seigo ase...@kde.org:
 On September 21, 2009, John Tapsell wrote:
 2009/9/20 Albert Astals Cid aa...@kde.org:
  Hi, not sure who's the responsible on that, blame says 73% of lines are
  from leonhard but CIA says he left KDE time ago, copyright mentions
  John so i'm mailing for John and plasma-devel (as replacement of
  leonhard).
 
  In plasma/widgets/signalplotter.cpp there is code like this

 Bah, reason number 412 why code copying is evil.  This is a copy of an
 old version.

 i can think of two approaches:

 a) make KSysGuard use QGraphicsView and share the widgets directly that way
 b) use a QGraphicsProxyWidget and encapsulate KSysGuard's classes that are
 used in libplasma

Do you know the advantages/disadvantages of those two approaches?  I
do want to get these merged.

Do I have to keep ABI/API compatibility now for plasma/widgets/signalplotter ?


 either way, it would be very nice to see KSysGuard using the plotter classes
 in libplasma (or libplasma using the same plotter classes as ksyguard, housed
 somewhere else) so we can get rid of this duplication altogether.

Which do you prefer?  I was thinking of moving the widget to
kdebase/workspace/libs   would this be sufficient?

John
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Bugs in signalplotter.cpp

2009-09-21 Thread John Tapsell
2009/9/20 Albert Astals Cid aa...@kde.org:
 Hi, not sure who's the responsible on that, blame says 73% of lines are from
 leonhard but CIA says he left KDE time ago, copyright mentions John so i'm
 mailing for John and plasma-devel (as replacement of leonhard).

 In plasma/widgets/signalplotter.cpp there is code like this

Bah, reason number 412 why code copying is evil.  This is a copy of an
old version.

I'll look into it.


    foreach (QListdouble data, d-plotData) {
        data.append(0);
    }


    foreach (QListdouble data, d-plotData) {
        if (newOrder.count() != data.count()) {
            kDebug()  Serious problem in move sample.  plotdata[i] has 
                      data.count()   and neworder has  
 newOrder.count();
        } else {
            QListdouble newPlot;
            for (int i = 0; i  newOrder.count(); i++) {
                int newIndex = newOrder[i];
                newPlot.append(data.at(newIndex));
            }
            data = newPlot;
        }
    }


    foreach (QListdouble data, d-plotData) {
        if ((uint)data.size() = pos) {
            data.removeAt(pos);
        }
    }



 This code does nothing, it is only working over temporary copies of the lists,
 the orginal ones are not modified, please read
 http://tsdgeos.blogspot.com/2008/04/qforeach-is-your-friend.html if you don't
 understand what i say.

 Albert

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel