Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-04 Thread Sujith H

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

(Updated 2009-04-04 00:52:01.302069)


Review request for Plasma.


Changes
---

I had made the nameDisplayOrder return ApplicationModel::DisplayOrder.


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-03 Thread Aaron Seigo


> On 2009-04-03 11:13:08, Sujith  H wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp,
> >  line 291
> > 
> >
> > I believe this method fits here. Is this method wrong?

the problem is that it returns a bool when it should return a DisplayOrder 
(look at what m_displayOrder is :)


- Aaron


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


On 2009-04-02 12:34:04, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 12:34:04)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-03 Thread Sujith H

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



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp


I believe this method fits here. Is this method wrong?


- Sujith 


On 2009-04-02 12:34:04, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 12:34:04)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Sujith H

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

(Updated 2009-04-02 12:34:04.446635)


Review request for Plasma.


Changes
---

I hope this time its ok :)


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Sujith H


> On 2009-04-02 10:06:27, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, line 
> > 611
> > 
> >
> > this should be:
> > 
> > return m_applicationModel->nameDisplayOrder() == 
> > ApplicationModel::NameBeforeDescription;
> 
>  wrote:
> nameDisplayOrder()? So that means I would have to write a function 
> nameDisplayOrder() function?
> 
>  wrote:
> yes: bool nameDisplayOrder() const;

so it should be :
bool nameDisplayOrder() const
{
   return ApplicationModel::NameBeforeDescription;
}
right?


- Sujith


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


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Shantanu Tushar Jha


> On 2009-04-02 11:55:26, Shantanu Tushar Jha wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, lines 
> > 596-597
> > 
> >
> > Maybe we can have a member variable m_showAppsByName which is used 
> > inside setupAllProgramsView() to set setNameDisplayOrder -
> > 
> > if(m_showAppsByName) {
> > 
> > applicationModel->setNameDisplayOrder(ApplicationModel::NameBeforeDescription);
> > } else {
> > applicationModel->setNameDisplayOrder(ApplicationModel::NameAfterDescription);
> > }
> > 
> > and Launcher::setShowAppsByName(bool showAppByName) just sets 
> > m_showAppsByName = showAppByName;
> > 
> > and bool Launcher::showAppsByName() returns m_showAppsByName
> > 
> > 
> > this will be better than making applicationModel as a private member, 
> > am I right Aaron?
> 
>  wrote:
> that won't work because the setting can be changed at runtime via the 
> settings dialog and after setupAllProgramsView is called, so you either need 
> to re-create the application model (rather wasteful) or else hang onto the 
> pointer to it.

ohkay, got it :)


- Shantanu


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


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Aaron Seigo


> On 2009-04-02 11:55:26, Shantanu Tushar Jha wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, lines 
> > 596-597
> > 
> >
> > Maybe we can have a member variable m_showAppsByName which is used 
> > inside setupAllProgramsView() to set setNameDisplayOrder -
> > 
> > if(m_showAppsByName) {
> > 
> > applicationModel->setNameDisplayOrder(ApplicationModel::NameBeforeDescription);
> > } else {
> > applicationModel->setNameDisplayOrder(ApplicationModel::NameAfterDescription);
> > }
> > 
> > and Launcher::setShowAppsByName(bool showAppByName) just sets 
> > m_showAppsByName = showAppByName;
> > 
> > and bool Launcher::showAppsByName() returns m_showAppsByName
> > 
> > 
> > this will be better than making applicationModel as a private member, 
> > am I right Aaron?

that won't work because the setting can be changed at runtime via the settings 
dialog and after setupAllProgramsView is called, so you either need to 
re-create the application model (rather wasteful) or else hang onto the pointer 
to it.


- Aaron


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


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Shantanu Tushar Jha

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



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


Maybe we can have a member variable m_showAppsByName which is used inside 
setupAllProgramsView() to set setNameDisplayOrder -

if(m_showAppsByName) {

applicationModel->setNameDisplayOrder(ApplicationModel::NameBeforeDescription);
} else {
applicationModel->setNameDisplayOrder(ApplicationModel::NameAfterDescription);
}

and Launcher::setShowAppsByName(bool showAppByName) just sets 
m_showAppsByName = showAppByName;

and bool Launcher::showAppsByName() returns m_showAppsByName


this will be better than making applicationModel as a private member, am I 
right Aaron?


- Shantanu


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Aaron Seigo


> On 2009-04-02 10:06:27, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, line 
> > 611
> > 
> >
> > this should be:
> > 
> > return m_applicationModel->nameDisplayOrder() == 
> > ApplicationModel::NameBeforeDescription;
> 
>  wrote:
> nameDisplayOrder()? So that means I would have to write a function 
> nameDisplayOrder() function?

yes: bool nameDisplayOrder() const;


> On 2009-04-02 10:06:27, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, line 
> > 596
> > 
> >
> > this will create a new ApplicationModel object, not modify the one that 
> > is actually in use already.
> > 
> > there is an ApplicationModel object created earlier in this file; that 
> > object should be kept in a member variable (e.g. m_applicationModel) and 
> > used here.
> 
>  wrote:
> There is a line "ApplicationModel *applicationModel = new 
> ApplicationModel(q);" in function:
> 
> void setupAllProgramsView()
> 
> Ahh this would be little tricky for me (in this learning phase :))

so you found where it is, that's good. now add:

ApplicationModel *applicationModel;

as a member of the Launcher::Private class, so that the line becomes:

applicationModel = new ApplicationModel(q);

then you can access it from Launcher via d->applicationModel :)


- Aaron


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


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Sujith H


> On 2009-04-02 10:06:27, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, line 
> > 611
> > 
> >
> > this should be:
> > 
> > return m_applicationModel->nameDisplayOrder() == 
> > ApplicationModel::NameBeforeDescription;

nameDisplayOrder()? So that means I would have to write a function 
nameDisplayOrder() function?


> On 2009-04-02 10:06:27, Aaron Seigo wrote:
> > /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp, line 
> > 596
> > 
> >
> > this will create a new ApplicationModel object, not modify the one that 
> > is actually in use already.
> > 
> > there is an ApplicationModel object created earlier in this file; that 
> > object should be kept in a member variable (e.g. m_applicationModel) and 
> > used here.

There is a line "ApplicationModel *applicationModel = new ApplicationModel(q);" 
in function:

void setupAllProgramsView()

Ahh this would be little tricky for me (in this learning phase :))


- Sujith


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


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Aaron Seigo

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


getting closer :)


/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp


this is still calling the wrong method. it shouldn't be calling 
switchTabsOnHover, it should be calling d->launcher->showAppsByName();



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


this will create a new ApplicationModel object, not modify the one that is 
actually in use already.

there is an ApplicationModel object created earlier in this file; that 
object should be kept in a member variable (e.g. m_applicationModel) and used 
here.



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


this should be:

return m_applicationModel->nameDisplayOrder() == 
ApplicationModel::NameBeforeDescription;



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


don't need this line.


- Aaron


On 2009-04-02 09:33:32, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-04-02 09:33:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-04-02 Thread Sujith H

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

(Updated 2009-04-02 09:33:32.533793)


Review request for Plasma.


Changes
---

Have a look at this diff :)


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-31 Thread Aaron Seigo

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



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h


this should be a private: member of the class. exposing variables directly 
isn't good, as it exposes the implementation details.



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp


if ((m_displayOrder == NameBeforeDescription) &&
(!node->genericName.isEmpty())) {
return node->genericName;
} else {
return node->appName;
}



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


this is an infinite loop :)

it needs to call the corresponding method in the apps model. e.g.:

if (showAppByName) {
m_applicationModel->setNameDisplayOrder(NameBeforeDescription);
} else {
m_applicationModel->setNameDisplayOrder(NameAfterDescription);
}

this implies that you will need to make the ApplicationModel a member of 
the class as well so it can be accessed at this point.



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp


this is an infinite loop :)

it needs to call the corresponding method in the apps model.


- Aaron


On 2009-03-31 11:24:45, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-03-31 11:24:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
>   
> /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
> 947244 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 947244 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-31 Thread Sujith H

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

(Updated 2009-03-31 11:24:45.801967)


Review request for Plasma.


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.h 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/ui/launcher.cpp 947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.cpp 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/core/applicationmodel.h 
947244 
  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 947244 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-30 Thread Sujith H


> On 2009-03-27 17:28:45, Aaron Seigo wrote:
> > great! a few small issues to take care of; the next step is now to read the 
> > value when the applet is created. that should go into void 
> > Launcher::setApplet(Plasma::Applet *applet) in ui/launcher.cpp. 
> > 
> > then modify ApplicationModel (in core/applicationmodel.cpp) to have the 
> > order set (e.g. setNameDisplayOrder(DisplayOrder) where DisplayOrder would 
> > be an enum (there's a FormatType enum in simpleapplet/simpleapplet.h that 
> > maybe could be moved somewhere in core/?), and then in 
> > ApplicationModel::data, return the name or generic name based on that 
> > setting.
> > 
> > that should complete the feature :)
> 
> Sujith  H wrote:
> I am stuck at ApplicationModel(in core/applicationmodel.cpp).
> 
> Aaron Seigo wrote:
> create a method that is something like void 
> setNameDisplayOrder(DisplayOrder) in ApplicationModel. then modify 
> ApplicationModel::data to return either the name or the description depending 
> on what the value was set to in setNameDisplayOrder. if you can describe 
> where you are stuck a bit more, perhaps i can offer a clearer answer.

I wrote a method in core/applicationmodel.cpp
int ApplicationModel::setNameDisplayOrder(DisplayOrder displayorder)
{
displayorder = NameBeforeDescription;
return displayorder;
}

In the core/application.h I wrote:

 class KICKOFF_EXPORT ApplicationModel : public KickoffAbstractModel
 {
 Q_OBJECT
Q_ENUMS(DisplayOrder)
public:
.
.
.
 enum DisplayOrder {
 NameAfterDescription,
 NameBeforeDescription
};
.
.
.
.
virtual int setNameDisplayOrder(DisplayOrder);
}
But I am confused in the switch statement. What will be role(i.e, switch(role) 
in core/applicationmodel.cpp)?
If I can know that I believe I can call setNameDisplayOrder(displayorder) there 
and return the value there.

I hope you got my point. If you need patch of my existing work, I can post that 
too.


- Sujith


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


On 2009-03-27 12:37:45, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-03-27 12:37:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 945188 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-30 Thread Aaron Seigo


> On 2009-03-27 17:28:45, Aaron Seigo wrote:
> > great! a few small issues to take care of; the next step is now to read the 
> > value when the applet is created. that should go into void 
> > Launcher::setApplet(Plasma::Applet *applet) in ui/launcher.cpp. 
> > 
> > then modify ApplicationModel (in core/applicationmodel.cpp) to have the 
> > order set (e.g. setNameDisplayOrder(DisplayOrder) where DisplayOrder would 
> > be an enum (there's a FormatType enum in simpleapplet/simpleapplet.h that 
> > maybe could be moved somewhere in core/?), and then in 
> > ApplicationModel::data, return the name or generic name based on that 
> > setting.
> > 
> > that should complete the feature :)
> 
> Sujith  H wrote:
> I am stuck at ApplicationModel(in core/applicationmodel.cpp).

create a method that is something like void setNameDisplayOrder(DisplayOrder) 
in ApplicationModel. then modify ApplicationModel::data to return either the 
name or the description depending on what the value was set to in 
setNameDisplayOrder. if you can describe where you are stuck a bit more, 
perhaps i can offer a clearer answer.


- Aaron


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


On 2009-03-27 12:37:45, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-03-27 12:37:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 945188 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-30 Thread Sujith H


> On 2009-03-27 17:28:45, Aaron Seigo wrote:
> > great! a few small issues to take care of; the next step is now to read the 
> > value when the applet is created. that should go into void 
> > Launcher::setApplet(Plasma::Applet *applet) in ui/launcher.cpp. 
> > 
> > then modify ApplicationModel (in core/applicationmodel.cpp) to have the 
> > order set (e.g. setNameDisplayOrder(DisplayOrder) where DisplayOrder would 
> > be an enum (there's a FormatType enum in simpleapplet/simpleapplet.h that 
> > maybe could be moved somewhere in core/?), and then in 
> > ApplicationModel::data, return the name or generic name based on that 
> > setting.
> > 
> > that should complete the feature :)

I am stuck at ApplicationModel(in core/applicationmodel.cpp).


- Sujith


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


On 2009-03-27 12:37:45, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-03-27 12:37:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 945188 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-27 Thread Aaron Seigo

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


great! a few small issues to take care of; the next step is now to read the 
value when the applet is created. that should go into void 
Launcher::setApplet(Plasma::Applet *applet) in ui/launcher.cpp. 

then modify ApplicationModel (in core/applicationmodel.cpp) to have the order 
set (e.g. setNameDisplayOrder(DisplayOrder) where DisplayOrder would be an enum 
(there's a FormatType enum in simpleapplet/simpleapplet.h that maybe could be 
moved somewhere in core/?), and then in ApplicationModel::data, return the name 
or generic name based on that setting.

that should complete the feature :)


/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp


please put each declaration on its own line:

QCheckBox *switchOnHoverCheckBox;
QCheckBox *appByNameCheckBox;



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp


the Launcher class (in ui/launcher.*) will need a bool showAppsByName() 
const; method in it to use here.



/trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp


minor niggle, but should probably be ShowAppsByName. hm... ListAppsByName 
might be even more natural.

though to future proof this, i'd probably use a string value and do 
something like:

cg.writeEntry("AppListingStyle", showAppByName ? "NameFirst" : 
"DescriptionFirst");

that way if we add more ways to list the apps later we don't have odd 
config files :)


- Aaron


On 2009-03-27 12:37:45, Sujith  H wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/441/
> ---
> 
> (Updated 2009-03-27 12:37:45)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
> section he/she can see "Audio Player - Amarok". But he requested it to be in 
> other way "Amarok - Audio Player". Hence as an initial step he asked me to 
> add a check box similar to "Switch tabs on hover". I had added a check box 
> for this. I am a newbie in KDE. Hence would like to know what next should I 
> do to accomplish the task.
> 
> 
> Diffs
> -
> 
>   /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 
> 945188 
> 
> Diff: http://reviewboard.kde.org/r/441/diff
> 
> 
> Testing
> ---
> 
> I had compiled the above patch in my build directory. It compiled without any 
> errors. 
> 
> 
> Thanks,
> 
> Sujith
> 
>

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-27 Thread Sujith H

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

(Updated 2009-03-27 12:37:45.873892)


Review request for Plasma.


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  /trunk/KDE/kdebase/workspace/plasma/applets/kickoff/applet/applet.cpp 945188 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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


Re: Review Request: Patch regarding IRC discussion with Aron Seigo(its related to the first step, for the task given to me)

2009-03-27 Thread Sujith H

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

(Updated 2009-03-27 12:28:22.628368)


Review request for Plasma.


Summary
---

As per Aron Seigo's suggestion in the launcher when one clicks the Multimedia 
section he/she can see "Audio Player - Amarok". But he requested it to be in 
other way "Amarok - Audio Player". Hence as an initial step he asked me to add 
a check box similar to "Switch tabs on hover". I had added a check box for 
this. I am a newbie in KDE. Hence would like to know what next should I do to 
accomplish the task.


Diffs (updated)
-

  
workspace/plasma/applets/kickoff/applet/workspace/plasma/applets/kickoff/applet/applet.cpp
 945188 

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


Testing
---

I had compiled the above patch in my build directory. It compiled without any 
errors. 


Thanks,

Sujith

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