Re: plasma-framework, kactivities and kactivities-stats: please consider proper de-KF-ication now

2023-11-05 Thread Shantanu Tushar Jha
Hi,

+1 to what Carl said. Right now my Elisa barely needs any time to read my
music library, presumably because Baloo already has it indexed. It'll be
quite a shame if apps did their own indexing, wasting time and power.

The effects (of removing Baloo support) are even more pronounced if we
consider that a majority of Elisa users are running Plasma as well.
(Citation needed, I know, but IMO it's not such a risky assumption.)

Regards,
Shantanu

On Sun, Nov 5, 2023, 11:53 PM Carl Schwan  wrote:

> On Sunday, November 5, 2023 6:01:38 PM CET Nate Graham wrote:
> > On 11/5/23 07:42, Kevin Ottens wrote:
> > > I was clumsily advocating for this Akademy 2021 or 2022 (can't remember
> > > which).
> > >
> > > This way it's clearer to application authors when they tie themselves
> to a
> > > given workspace or not.
> > >
> > > Also, isn't Elisa able to work without Baloo? It even seems to do the
> > > right
> > > thing if I trust its CMakeLists.txt. It has Baloo as a recommended but
> > > optional dependency, and disable it altogether for Win32 builds.
> >
> > Yes, Elisa also includes an internal indexer, for use on Windows and
> > Android, or on *Nix when Baloo isn't installed or is disabled.
> >
> > I think the original idea for the app was to delegate all the indexing
> > heavy lifting to Baloo to avoid internal complication, but clearly this
> > has not worked out in practice, since to be truly cross-platform, it
> > can't assume that Baloo is present and active and does need its own
> > indexer. So maybe the best course of action is actually to remove Baloo
> > support entirely and always use the internal indexer, so that we don't
> > have two different code paths.
> >
> > Nate
>
> I do not belive this is a good idea. Mostly because it means we would then
> have an indexer in Baloo and one indexer in Elisa running at the same time
> and
> doing something similar which is wasteful. It becomes even more of an
> issue
> when other apps like Peruse or Arianna start doing the same and use their
> own
> indexer instead of Baloo on Plasma.
>
> This defeat the point of having a general purpose indexer in Plasma.
>
> Cheers,
> Carl
>
>
>
>


Re: Review Request 118697: Treat albums with different artists as different albums

2016-09-22 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118697/
---

(Updated Sept. 22, 2016, 8:40 p.m.)


Status
--

This change has been discarded.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

This one has been pending for some time now. Our Album data model assumed that 
a album name is enough to uniquely identify an album. This creates problems 
because multiple artists may have albums with the same name.
This patch makes Media Library and ItemCache aware of this fact.


Diffs
-

  libs/mediacenter/album.h 4926e48 
  libs/mediacenter/album.cpp 881f65a 
  libs/mediacenter/itemcache.h ffee05f 
  libs/mediacenter/medialibrary.cpp 9c58b44 
  libs/test/itemcachetest.h 4fc6807 
  libs/test/itemcachetest.cpp e6dee33 
  libs/test/medialibrarytest.h 718457d 
  libs/test/medialibrarytest.cpp e1a8c9b 
  libs/test/mediatest.cpp 8fde60b 
  libs/test/pmcalbumtest.cpp 9f2b4c8 

Diff: https://git.reviewboard.kde.org/r/118697/diff/


Testing
---

Unit tests pass. Added test to take care of the new scenario.


Thanks,

Shantanu Tushar



Re: Moving plasma-mediacenter to extragear

2016-07-18 Thread Shantanu Tushar Jha
Hi,

On Mon, Jul 18, 2016 at 2:29 PM, Bhushan Shah <bhus...@gmail.com> wrote:

> Hello plasma team,
>
> I am not much happy to write this, but here it goes,
>
> Since last three Plasma release it always bugged me that there were no
> usefull changes in plasma-mediacenter, mostly because after being
> contracted by Bluesystems I got involved into Plasma Mobile related
> work and didn't manage to find time to work on plasma-mediacenter.
>
> I already have some work branches in plasma-mediacenter which enables
> the containment which can work nicely with remote controller, and other
> local work branch for medialibrary improvements. However with the speed
> plasma releases are happening I am too hesistent to merge those as it
> would break things and I may or may not get time to fix those in time.
>

I kinda had this feeling when the PMC move to a core Plasma component which
needs more cautious development. It was always easier to work on PMC
treating it as a standalone component, so +1 from me.


> So therefore, I am proposing to move plasma-mediacenter to extragear or
> playground and not release it with Plasma/5.8 anymore. In extragear or
> playground I will be able to define my own release scedule and cycle,
> and merge my changes.
>
> If there are no objections within next week, I will file a sysadmin
> request to move plasma-mediacenter to extragear/playground and request
> Riddell to not release it anymore with Plasma/5.8.
>
> Thanks
>

> --
> Bhushan Shah
>
> http://bhush9.github.io
> IRC Nick : bshah on Freenode
>
> ___
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>



-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 125993: Implemented lastfmimagefetcher as a Plugin

2015-11-23 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125993/#review88715
---


Do the tests not work anymore? Just curious why were they removed.


libs/mediacenter/lastfmimagefetcher.cpp (line 155)
<https://git.reviewboard.kde.org/r/125993/#comment60828>

remove the extra whitespace


- Shantanu Tushar


On Nov. 22, 2015, 2:23 p.m., Aditya Dev Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125993/
> ---
> 
> (Updated Nov. 22, 2015, 2:23 p.m.)
> 
> 
> Review request for Plasma, Ashish Bansal and Bhushan Shah.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> ---
> 
> Implemented it like the other datasources.
> 
> Signal newMedia from MediaLibrary is absorbed by handleLastFmNewMedia . 
> handleLastFmNewMedia then calls fetchImage for each newMedia in the 
> newMediaList. Created a QList for storing the new media.
> 
> After the image is fetched, gotImage emits the signal imageFetched (from 
> mediaLibrary) with the QList we created earlier. This signal is absorbed by 
> handleNewMedia in PmcMetaDataModel which does the rest.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 04ba1c3 
>   autotests/lastfmimagefetchertest.h eb7cb29 
>   autotests/lastfmimagefetchertest.cpp abb3fd6 
>   datasources/CMakeLists.txt a7171b7 
>   datasources/lastfm/CMakeLists.txt PRE-CREATION 
>   datasources/lastfm/lastfmimagefetcher.desktop PRE-CREATION 
>   libs/mediacenter/CMakeLists.txt 5a13449 
>   libs/mediacenter/lastfmimagefetcher.h ecff37e 
>   libs/mediacenter/lastfmimagefetcher.cpp e005077 
>   libs/mediacenter/medialibrary.h 45744c1 
>   libs/mediacenter/pmcmetadatamodel.cpp 2fe6efe 
> 
> Diff: https://git.reviewboard.kde.org/r/125993/diff/
> 
> 
> Testing
> ---
> 
> Builds 100%.
> However the audio scrobbler api doesn't seem to work which I'll fix after 
> this. 
> Also, I get the following Output : http://ix.io/mps
> Apart from it, all the datasources are loaded and media plays as expected.
> 
> 
> Thanks,
> 
> Aditya Dev Sharma
> 
>

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


Re: Review Request 123467: Add CoverArt uri to the metadata exposed with MPRIS interfaces

2015-04-22 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123467/#review79349
---


The idea sounds, some nitpicks though.


libs/mediacenter/mpris2/mpris2.cpp (line 33)
https://git.reviewboard.kde.org/r/123467/#comment54195

Hardcoding '/' might not be portable, try using QDir's features to get what 
you want. I'd prefer having a class QDir var which is initialized to the full 
path (till covers/) in the ctor.



libs/mediacenter/mpris2/mpris2.cpp (line 104)
https://git.reviewboard.kde.org/r/123467/#comment54197

Make this a member as suggested above.



libs/mediacenter/mpris2/mpris2.cpp (lines 106 - 108)
https://git.reviewboard.kde.org/r/123467/#comment54196

Using the QDir member as suggested above, call absoluteFilePath instead of 
doing a string concatenate.


- Shantanu Tushar


On April 22, 2015, 1:44 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123467/
 ---
 
 (Updated April 22, 2015, 1:44 p.m.)
 
 
 Review request for Plasma, Bhushan Shah, Shantanu Tushar, and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Save Cover art to a temperoray folder. Include the path of the cover art in 
 the metadata currently exposed with MPRIS interfaces.
 
 
 Diffs
 -
 
   libs/mediacenter/mpris2/mpris2.cpp f03d062 
 
 Diff: https://git.reviewboard.kde.org/r/123467/diff/
 
 
 Testing
 ---
 
 Correct path is showing up in the metadata when tested with mpristester.
 Correct cover art is showing up in the mpris controller (appearing in system 
 tray) of Plasma 5.
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: plasma-mediacenter in kdereview from extragear

2015-03-06 Thread Shantanu Tushar Jha
On Sat, Mar 7, 2015 at 3:19 AM, Albert Astals Cid aa...@kde.org wrote:

 El Dijous, 5 de març de 2015, a les 08:51:37, Bhushan Shah va escriure:
  Hello!
 
  On Thu, Feb 19, 2015 at 8:22 PM, Bhushan Shah bhus...@gmail.com wrote:
   We want to move plasma-mediacenter to kde/workspace for releasing it
   with Plasma 5.x release cycle, Plasma media center is living room
   solution of plasma workspaces, it is integrated as shell package for
   PMC.
 
  Given the two week has been passed in kdereview and as there are no
  objections I will file the sysadmin request to move the repository to
  kde/workspace

 Is there ever going to be another release of the stable kdelibs4-based 1.4
 branch? I'm asking because it used to be in extragear and moved to
 workspace,
 and thus the l10n of the kde4 branch needs some engineering to still be in
 extragear, and it'd be just easier for us if you're not planning more
 releases
 since then we can just remove the l10n handlong in kde4 land altogether.


No, the kdelibs4 based version will not see a new release. We are focusing
all our efforts on the kf5/Plasma5 version now.

Cheers,

-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com



 Cheers,
   Albert

 
  Thanks!


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


Re: Review Request 122745: Monitor baloo for new media files

2015-02-27 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122745/#review76752
---

Ship it!


Ship It!

- Shantanu Tushar


On Feb. 27, 2015, 4:47 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/122745/
 ---
 
 (Updated Feb. 27, 2015, 4:47 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Vishesh Handa.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This is uses the dbus signal provided by baloo, checks them mimetype of that 
 file and passes that to related searchresulthandler
 
 Bonus point of this is in case properties of any files changes we also get 
 updates.
 
 
 Diffs
 -
 
   mediasources/baloosearch/CMakeLists.txt 
 9bec5fd6c5af585b8faa53d5440878bf0dcf6566 
   mediasources/baloosearch/audiosearchresulthandler.h 
 3efb4bf1efc337a0a293bc1c391707302a76172c 
   mediasources/baloosearch/audiosearchresulthandler.cpp 
 0c9d93b1988af62836f9c547707395810da311a2 
   mediasources/baloosearch/baloosearchmediasource.h 
 e0289026e2cbf9c7e145f66b057b7ae6287b8e48 
   mediasources/baloosearch/baloosearchmediasource.cpp 
 b90e9c03e352d1fd6962fbb28de9b8714e866fde 
   mediasources/baloosearch/imagesearchresulthandler.h 
 72a2cb30617b91b92e9176688e21d63504876eb1 
   mediasources/baloosearch/imagesearchresulthandler.cpp 
 6df3a2c11092422a18b00bb668498dc1b5fcbc2e 
   mediasources/baloosearch/searchresulthandler.h 
 176327409fa9acd92d21a69e2a91f035d9140f51 
   mediasources/baloosearch/videosearchresulthandler.h 
 00d048cc0bfa44ae323afd531c7b41a3bcbcd528 
   mediasources/baloosearch/videosearchresulthandler.cpp 
 f1439e735be43be68d0b4625019493f9249ae1e5 
 
 Diff: https://git.reviewboard.kde.org/r/122745/diff/
 
 
 Testing
 ---
 
 Moved some files from External drive to $HOME, they got added to 
 medialibrary, verified by debug messages for images and manually for music 
 files
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 121144: Remove MetadaBackendCommonModel

2014-11-16 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121144/#review70498
---

Ship it!


Ship It!

- Shantanu Tushar


On Nov. 17, 2014, 5:55 a.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121144/
 ---
 
 (Updated Nov. 17, 2014, 5:55 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 this was just a wrapper around PmcMetadataModel and was not doing anything
 
 
 Diffs
 -
 
   browsingbackends/metadatavideobackend/metadatavideomodel.h 3e85f4a 
   browsingbackends/metadatavideobackend/metadatavideomodel.cpp 417cddd 
   browsingbackends/metadatabackendcommonmodel.h 0873bc0 
   browsingbackends/metadatabackendcommonmodel.cpp fbee1ff 
   browsingbackends/metadatamusicbackend/CMakeLists.txt 48e98ea 
   browsingbackends/metadatamusicbackend/metadatamusicbackend.h 836ee21 
   browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp d9f4fa6 
   browsingbackends/metadatamusicbackend/metadatamusicsongsmodel.h 53efbd2 
   browsingbackends/metadatamusicbackend/metadatamusicsongsmodel.cpp 55debf0 
   browsingbackends/metadatavideobackend/CMakeLists.txt 2b81ab2 
   browsingbackends/metadatavideobackend/metadatavideobackend.cpp 1ed833d 
 
 Diff: https://git.reviewboard.kde.org/r/121144/diff/
 
 
 Testing
 ---
 
 everything works fine without it
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 121130: Remove okToLoad method of backend

2014-11-15 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121130/#review70417
---

Ship it!


Ship It!

- Shantanu Tushar


On Nov. 15, 2014, 4:12 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/121130/
 ---
 
 (Updated Nov. 15, 2014, 4:12 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This was case for local filebrowsing on plasma active but since we already 
 removed localfile browsing backends remove this method
 
 I cross checked this with other backends that might need such use case but I 
 think those backends can return false in initImpl or choose not to give any 
 data
 
 
 Diffs
 -
 
   libs/mediacenter/abstractbrowsingbackend.h 3a9dbea 
   libs/mediacenter/abstractbrowsingbackend.cpp f770759 
   libs/mediacenter/backendsmodel.cpp a4fff82 
 
 Diff: https://git.reviewboard.kde.org/r/121130/diff/
 
 
 Testing
 ---
 
 compiles fine
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 120980: Proper API for Play all function

2014-11-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120980/#review69831
---

Ship it!


Looks good, some minor fixes and this can go in.


browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp
https://git.reviewboard.kde.org/r/120980/#comment48875

pull m_musicFilteredModel-index(i, 0) into a variable



browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp
https://git.reviewboard.kde.org/r/120980/#comment48876

use auto



libs/mediacenter/abstractbrowsingbackend.cpp
https://git.reviewboard.kde.org/r/120980/#comment48874

return QStringList();


- Shantanu Tushar


On Nov. 4, 2014, 1:44 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120980/
 ---
 
 (Updated Nov. 4, 2014, 1:44 p.m.)
 
 
 Review request for Plasma, Bhushan Shah, Shantanu Tushar, Sinny Kumari, and 
 Sujith Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 - AbstractBrowsingBackend::allMedia() returns all media in QStringList
 - PlaylistModel::addAllToPlaylist() to add QStringList in current playlist
 - This all is handled by mediabrowser and playlist.qml
 
 
 Diffs
 -
 
   browsingbackends/metadatamusicbackend/metadatamusicbackend.h 
 c9f10a77753f7427f669da9cb5e46cf66994f57d 
   browsingbackends/metadatamusicbackend/metadatamusicbackend.cpp 
 f4f908047586694188a354bc69ecc49824f15fe7 
   libs/mediacenter/abstractbrowsingbackend.h 
 910272a93ec047418ad1a4c6534592bb20fcba60 
   libs/mediacenter/abstractbrowsingbackend.cpp 
 c24d5178d91a09b4e9f8b5b74d7a88616a8b9756 
   libs/mediacenter/playlistmodel.h d45611c1eaddb657ce3ed97731e9e70990b7a060 
   libs/mediacenter/playlistmodel.cpp 501e501888d53fd0fdd6574a72988408307b3f7b 
   mediaelements/mediabrowser/MediaBrowser.qml 
 97942881d08c12539675ba08d7fc6a8b28206fb5 
   mediaelements/playlist/Playlist.qml 
 f39be2a8a695499fc05d69cd0c13d70e18356142 
   shells/next/contents/views/Desktop.qml 
 8ed3d1e2e013cbfd0aac9d0fca099901fac3f237 
 
 Diff: https://git.reviewboard.kde.org/r/120980/diff/
 
 
 Testing
 ---
 
 works fine
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 120855: Add Google two-step verification support to PMC Picasa login

2014-11-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120855/#review69698
---


Thanks for your time on this, good stuff :) Tried it and the only caveat is 
that after login is done I have to press the back button (the one on the pmc 
controller) to get my album list. That should be fixed.


browsingbackends/onlineservices/picasa/picasamodel.h
https://git.reviewboard.kde.org/r/120855/#comment48822

where do you get these from?


- Shantanu Tushar


On Nov. 2, 2014, 3:34 p.m., Ashish Bansal wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120855/
 ---
 
 (Updated Nov. 2, 2014, 3:34 p.m.)
 
 
 Review request for KDEPIM, KDEPIM-Libraries, Plasma, Bhushan Shah, Shantanu 
 Tushar, Sinny Kumari, and Sujith Haridasan.
 
 
 Bugs: 336398
 http://bugs.kde.org/show_bug.cgi?id=336398
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Changed auth method from Client Login to OAuth using libkgapi2
 
 
 Diffs
 -
 
   browsingbackends/onlineservices/picasa/CMakeLists.txt bd5052d 
   browsingbackends/onlineservices/picasa/picasabackend.h 7144dc7 
   browsingbackends/onlineservices/picasa/picasabackend.cpp 6317240 
   browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml 
 e8f97f7 
   browsingbackends/onlineservices/picasa/picasacomponents/qmldir c790310 
   browsingbackends/onlineservices/picasa/picasamodel.h 5bce055 
   browsingbackends/onlineservices/picasa/picasamodel.cpp 2b64156 
 
 Diff: https://git.reviewboard.kde.org/r/120855/diff/
 
 
 Testing
 ---
 
 Everything is working fine except that after successful login, user has to 
 press back to view all the albums.
 
 
 Thanks,
 
 Ashish Bansal
 


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


Plasma Media Center and external USB drives

2014-10-23 Thread Shantanu Tushar Jha
Hi folks,

While we are busy implementing the new PMC design[1], we revisited the
question on what happens to USB devices. Finally we think we will offer the
following options to the user when a USB device is detected-

1. Mount the USB device and ask Baloo to index it so that the media show up
in the usual PMC (Video/Songs/Pictures) screens. (This option might not be
preferred if the user, for some reason, doesn't want the drive to be
indexed and be available to other apps as well)
2. Mount the USB device and let the user browse through files and play them
manually.

Just like there are two options, we have two questions that we needed some
usability thinking around-

1. Do these two options make sense? Or can someone think of even better
ways of handling this?
2. How (and where) do we display the options to the user, in the new mockup?

Ideas?

[1] https://forum.kde.org/viewtopic.php?f=285t=121705#p316799

-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120298: Enable lastfmimagefetchertest again,

2014-09-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120298/#review67100
---



libs/mediacenter/lastfmimagefetcher.cpp
https://git.reviewboard.kde.org/r/120298/#comment46833

In addition to this, can you qDebug the error message so that when this 
fails in non-test scenarios, there is something to debug.


- Shantanu Tushar


On Sept. 20, 2014, 3:04 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120298/
 ---
 
 (Updated Sept. 20, 2014, 3:04 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This enables lastfmimagefetchertest again with error handling
 
 
 Diffs
 -
 
   libs/mediacenter/lastfmimagefetcher.h 25d53fd 
   libs/mediacenter/lastfmimagefetcher.cpp 8d9edf4 
   libs/test/lastfmimagefetchertest.h f315f7c 
   libs/test/lastfmimagefetchertest.cpp e9ff4f1 
 
 Diff: https://git.reviewboard.kde.org/r/120298/diff/
 
 
 Testing
 ---
 
 Checked without internet, test is getting skipped.
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 119218: Add option to toggle between PerspectiveCrop/Fit while viewing images

2014-09-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119218/#review67101
---

Ship it!


Ship It!

- Shantanu Tushar


On Sept. 20, 2014, 1:59 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119218/
 ---
 
 (Updated Sept. 20, 2014, 1:59 p.m.)
 
 
 Review request for Plasma, Bhushan Shah, Shantanu Tushar, and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Double clicking the image adds toggles between PerspectiveCrop/Fit. This 
 feature is particularly useful while viewing landscape images. 
 
 
 Diffs
 -
 
   mediaelements/imageviewer/ImageViewer.qml 
 81f9cc6a9268040e657216d1dee2e00abf76b4fa 
 
 Diff: https://git.reviewboard.kde.org/r/119218/diff/
 
 
 Testing
 ---
 
 Open new image, double click -- toggles. double click again -- toggles 
 again. 
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 120298: Enable lastfmimagefetchertest again,

2014-09-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120298/#review67102
---

Ship it!


Other than that, this is okay to go in.

- Shantanu Tushar


On Sept. 20, 2014, 3:04 p.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120298/
 ---
 
 (Updated Sept. 20, 2014, 3:04 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This enables lastfmimagefetchertest again with error handling
 
 
 Diffs
 -
 
   libs/mediacenter/lastfmimagefetcher.h 25d53fd 
   libs/mediacenter/lastfmimagefetcher.cpp 8d9edf4 
   libs/test/lastfmimagefetchertest.h f315f7c 
   libs/test/lastfmimagefetchertest.cpp e9ff4f1 
 
 Diff: https://git.reviewboard.kde.org/r/120298/diff/
 
 
 Testing
 ---
 
 Checked without internet, test is getting skipped.
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 118866: Start playing after Play All is used

2014-08-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118866/
---

(Updated Aug. 4, 2014, 4:01 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

We should ask the model to start playing once we add all the songs to the 
playlist as a result of the user using Play All.


Diffs
-

  libs/mediacenter/multipleplaylistmodel.h 1c7731b 
  libs/mediacenter/multipleplaylistmodel.cpp 950675e 
  libs/mediacenter/playlistmodel.h 50157ff 
  libs/mediacenter/playlistmodel.cpp 44abbc1 
  browsingbackends/localfiles/localfilesabstractbackend.cpp bbd511b 
  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
8b8e149 

Diff: https://git.reviewboard.kde.org/r/118866/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.

2014-08-01 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118406/#review63646
---



mediaelements/mediaplayer/MediaPlayer.qml
https://git.reviewboard.kde.org/r/118406/#comment44341

Video should not know about playlistModel



mediaelements/playlist/Playlist.qml
https://git.reviewboard.kde.org/r/118406/#comment44342

what is this for? it won't do anything


- Shantanu Tushar


On Aug. 1, 2014, 4:03 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118406/
 ---
 
 (Updated Aug. 1, 2014, 4:03 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 333764
 http://bugs.kde.org/show_bug.cgi?id=333764
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 If a media(in a playlist) is located in an inaccessible location, then the 
 user is notified about the same. 
 
 
 Diffs
 -
 
   mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c 
   mediaelements/playlist/Playlist.qml 5dde297 
 
 Diff: https://git.reviewboard.kde.org/r/118406/diff/
 
 
 Testing
 ---
 
 1. Load media to a playlist.
 2. Unmount the device containing media.
 3. Check if the user is notified of the location being inaccessible
--yes, the user is notified
 4. Mount the device containing media and play a media from playlist.
-- The media plays properly.
 
 
 File Attachments
 
 
 wihtout_i18n.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/06/05/3dc148a5-c4da-4d27-a713-e63922cbcef8__wihtout_i18n.png
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Reviews

2014-07-29 Thread Shantanu Tushar Jha
This one needs to be discarded as well,
https://git.reviewboard.kde.org/r/115865/


On Tue, Jul 29, 2014 at 5:15 PM, Bhushan Shah bhus...@gmail.com wrote:

 Hello,

 On Tue, Jul 29, 2014 at 3:37 PM, David Edmundson
 da...@davidedmundson.co.uk wrote:
 
  There are a /lot/ of Plasma Media Centre reviews open (about half are
  PMC), I'm not sure what the state of those is, are they awaiting
  reviews from the non-media centre devs, or just no longer relevant. If
  someone from PMC pings me I can help go through and discard outdated
  things.

 Following PMC reviews needs to be discarded,

 https://git.reviewboard.kde.org/r/116460/ (Fixed already)
 https://git.reviewboard.kde.org/r/116804/ (Ditto)
 https://git.reviewboard.kde.org/r/116731 (I think, it was discarded
 long ago, I don't know how it came back? bug was also closed)

 I'll let you know as I will go through other reviews. Looks like there
 is no way to filter search by repo name or am I missing something.?

 Lets see..

 --
 Bhushan Shah

 http://bhush9.github.io
 IRC Nick : bshah on Freenode
 ___
 Plasma-devel mailing list
 Plasma-devel@kde.org
 https://mail.kde.org/mailman/listinfo/plasma-devel




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118866: Start playing after Play All is used

2014-07-29 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118866/
---

(Updated July 29, 2014, 12:35 p.m.)


Review request for Plasma.


Changes
---

Consistent behavior for Local Browsing backends as well


Repository: plasma-mediacenter


Description
---

We should ask the model to start playing once we add all the songs to the 
playlist as a result of the user using Play All.


Diffs (updated)
-

  libs/mediacenter/multipleplaylistmodel.h 1c7731b 
  libs/mediacenter/multipleplaylistmodel.cpp 950675e 
  libs/mediacenter/playlistmodel.h 50157ff 
  libs/mediacenter/playlistmodel.cpp 44abbc1 
  browsingbackends/localfiles/localfilesabstractbackend.cpp bbd511b 
  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
8b8e149 

Diff: https://git.reviewboard.kde.org/r/118866/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 116898: Get rid of unnecessary Q_INVOKABLE declarations

2014-07-29 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116898/
---

(Updated July 29, 2014, 1:19 p.m.)


Status
--

This change has been discarded.


Review request for Plasma and Sujith Haridasan.


Repository: plasma-mediacenter


Description
---

Methods supposed to be used as READ of Q_PROPERTY properties need not be 
invokable. Same goes for slots.


Diffs
-

  libs/mpris2/mediaplayer2.h e68bc5c 
  libs/mpris2/mediaplayer2player.h 203d681 
  libs/mpris2/mpris2.h 0df64f4 
  libs/mpris2/mpris2.cpp a8ad3ef 
  shells/newshell/main.cpp bab6915 
  shells/newshell/mainwindow.cpp d2d71d4 

Diff: https://git.reviewboard.kde.org/r/116898/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 119250: Events and trips according to date ranges

2014-07-22 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

(Updated July 22, 2014, 3:28 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs
-

  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 
  browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a 
  browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml 
d15a46e 
  libs/mediacenter/abstractbrowsingbackend.h 14ad502 
  libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 
  shells/newshell/package/contents/ui/mediacenter.qml 15c9cee 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


File Attachments


Showing events
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
Showing photos from an event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
Editing dates of an existing event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png


Thanks,

Shantanu Tushar

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


Re: Review Request 119250: Events and trips according to date ranges

2014-07-20 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

(Updated July 20, 2014, 3:08 p.m.)


Review request for Plasma.


Changes
---

Fix some UI bugs


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs (updated)
-

  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 
  browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a 
  browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml 
d15a46e 
  libs/mediacenter/abstractbrowsingbackend.h 14ad502 
  libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 
  shells/newshell/package/contents/ui/mediacenter.qml 15c9cee 
  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


File Attachments


Showing events
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
Showing photos from an event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
Editing dates of an existing event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png


Thanks,

Shantanu Tushar

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


Re: Review Request 119250: Events and trips according to date ranges

2014-07-20 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

(Updated July 20, 2014, 5:14 p.m.)


Review request for Plasma.


Changes
---

Prevent empty eventName or an existing eventName when not editing
Also added keyboard trigerring


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs (updated)
-

  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 
  browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a 
  browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml 
d15a46e 
  libs/mediacenter/abstractbrowsingbackend.h 14ad502 
  libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 
  shells/newshell/package/contents/ui/mediacenter.qml 15c9cee 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


File Attachments


Showing events
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
Showing photos from an event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
Editing dates of an existing event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png


Thanks,

Shantanu Tushar

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


Re: Review Request 119250: Events and trips according to date ranges

2014-07-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

(Updated July 19, 2014, 2:37 p.m.)


Review request for Plasma.


Changes
---

Fix bugs noted in the reviews. Note that this no longer lets the user edit the 
name of an existing event.


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs (updated)
-

  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  browsingbackends/onlineservices/picasa/picasabackend.h 83b3eb2 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/onlineservices/picasa/picasabackend.cpp b01ba4a 
  browsingbackends/onlineservices/picasa/picasacomponents/PicasaSidePanel.qml 
d15a46e 
  libs/mediacenter/abstractbrowsingbackend.h 14ad502 
  libs/mediacenter/abstractbrowsingbackend.cpp 6cd32f8 
  shells/newshell/package/contents/ui/mediacenter.qml 15c9cee 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  mediaelements/mediabrowser/MediaBrowser.qml b4ebdf9 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


File Attachments


Showing events
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
Showing photos from an event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
Editing dates of an existing event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png


Thanks,

Shantanu Tushar

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


Re: Plasma shell - Mediacenter

2014-07-19 Thread Shantanu Tushar Jha
Hi,

On Mon, Jul 14, 2014 at 2:16 PM, Marco Martin notm...@gmail.com wrote:

 On Saturday 12 July 2014, Shantanu Tushar Jha wrote:
  Hi folks,
 
  The frameworks branch of PMC now contains a Plasma Shell which basically
  loads the required QML components and looks/works exactly the same[1]. So
  technically we are good to go if a user wants her desktop to be a media
  center.
 
  However, it'll be nice if we can discuss on how the actual workflow could
  be for a user. Here are some things I can imagine-
 
  * User can have a mediacenter activity which she can switch to for
 getting
  the multimedia experience.

 I'm not sure how much it can work as activity,
 if you want that tough most o the mediacenter UI QML should be moved in  a
 single containment (that may be good as well) while now is iirc mostly in
 the
 view of the desktop package


Do you mean that the current implementation won't support it, or the idea
itself is not good?




  * Or, maybe she can configure the mediacenter shell to activate whenever
 an
  external TV screen is connected (through HDMI or something).

 it could either switch the shell or just launch the standalone application
 (launching the application should be easy, maybe configurable from kscreen
 kcm?)


Or, maybe plasmashell exposes a D-Bus API that can load shells on the fly?

+1 for the kscreen kcm, I think it should be a nice feature in general.



  * The device is a dedicated mediacenter device - the user tells Plasma to
  always load mediacenter instead of deskop

 I think there should eventually be a session shipped by it so is just
 possible
 to login into it from the login manager


Guess the session will always run the same startkde script, how can we tell
plasmashell (rhymes :P) what to load?



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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Extending fullscreen option to all pages in PMC

2014-07-15 Thread Shantanu Tushar Jha
KWin has a standard Ctrl+Shift+F shortcut to do that, but somehow it
doesn't work. Maybe you can try and figure out why.


On Tue, Jul 15, 2014 at 4:51 PM, Devanshu Jain devanshu.jain...@gmail.com
wrote:

 Hi guys!
 I was wondering if it would be nice that the user can switch at any time
 to full screen, by pressing a key instead of going to home screen every
 time and going to settings menu.

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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 119250: Events and trips according to date ranges

2014-07-14 Thread Shantanu Tushar


 On July 12, 2014, 7:39 p.m., R.Harish  Navnit wrote:
  I liked the interface and the idea, +1  for that.
  
  Some of the issues that I faced are(brief testing):
  
  1. Create an event - Exit PMC - add new event - unable to do so :(
  2. [Logical] Was able to create an event/trip from October to July
  3. Something that I couldn't figure out, how do I add content to the 
  trip/event folders ?
 Edit option only allows me to change the timeline, should have an 
  option to add/remove images. 
  
  Otherwise, this would be nice feature to have in PMC. Besides, 2 would be a 
  junior job that someone else could attempt to fix, not critical but pointed 
  it out nevertheless.

1, not sure what you mean, I can create an event, exit pmc and can add more 
events as well, works fine.
2. good point, I should have a check for that. Will upload a fresh patch.
3. well the idea is that you dont manually add images (that you can anyway do 
using folders), but yes we can have an option of excluding some images.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/#review62215
---


On July 12, 2014, 6:13 p.m., Shantanu Tushar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119250/
 ---
 
 (Updated July 12, 2014, 6:13 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 I got this idea on a recent trip that usually a family would have a device 
 where they store their trips/events photos on. Easier than tagging 
 everything, what if you could just say Hey, I went to Kashmir during 5 April 
 to 14 April 2014 and voila!
 
 This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
 to hear some feedback around this :)
 
 
 Diffs
 -
 
   browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp 
 PRE-CREATION 
   browsingbackends/metadatabackends/CMakeLists.txt ae56603 
   browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
 PRE-CREATION 
   libs/mediacenter/backendsmodel.cpp 2823fb6 
   libs/mediacenter/filtermediamodel.h b8272f6 
   browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventsbackend.h 
 PRE-CREATION 
   
 browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml
  PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
 PRE-CREATION 
   
 browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
  PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
 PRE-CREATION 
   browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/119250/diff/
 
 
 Testing
 ---
 
 Seems to work as expected, I will be attaching screenshots.
 
 
 File Attachments
 
 
 Showing events
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
 Showing photos from an event
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
 Editing dates of an existing event
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png
 
 
 Thanks,
 
 Shantanu Tushar
 


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


Plasma shell - Mediacenter

2014-07-12 Thread Shantanu Tushar Jha
Hi folks,

The frameworks branch of PMC now contains a Plasma Shell which basically
loads the required QML components and looks/works exactly the same[1]. So
technically we are good to go if a user wants her desktop to be a media
center.

However, it'll be nice if we can discuss on how the actual workflow could
be for a user. Here are some things I can imagine-

* User can have a mediacenter activity which she can switch to for getting
the multimedia experience.

* Or, maybe she can configure the mediacenter shell to activate whenever an
external TV screen is connected (through HDMI or something).

* The device is a dedicated mediacenter device - the user tells Plasma to
always load mediacenter instead of deskop

Any other ideas? Thoughts?

[1] http://i.imgur.com/J16niw7.jpg

Cheers!

-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 119250: Events and trips according to date ranges

2014-07-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs
-

  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


Thanks,

Shantanu Tushar

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


Re: Review Request 119250: Events and trips according to date ranges

2014-07-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119250/
---

(Updated July 12, 2014, 6:13 p.m.)


Review request for Plasma.


Changes
---

Adding screenshots


Repository: plasma-mediacenter


Description
---

I got this idea on a recent trip that usually a family would have a device 
where they store their trips/events photos on. Easier than tagging everything, 
what if you could just say Hey, I went to Kashmir during 5 April to 14 April 
2014 and voila!

This backend implements this idea in a (hopefully) easy-to-use way. I'll love 
to hear some feedback around this :)


Diffs
-

  browsingbackends/metadatabackends/eventsbackend/eventsmodel.cpp PRE-CREATION 
  browsingbackends/metadatabackends/CMakeLists.txt ae56603 
  browsingbackends/metadatabackends/eventsbackend/CMakeLists.txt PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventspicturesmodel.cpp 
PRE-CREATION 
  libs/mediacenter/backendsmodel.cpp 2823fb6 
  libs/mediacenter/filtermediamodel.h b8272f6 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.desktop 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsbackend.h PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/DatePicker.qml 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/Digit.qml 
PRE-CREATION 
  
browsingbackends/metadatabackends/eventsbackend/eventscomponents/EventsConfiguration.qml
 PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventscomponents/qmldir 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.h 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsfiltermodel.cpp 
PRE-CREATION 
  browsingbackends/metadatabackends/eventsbackend/eventsmodel.h PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/119250/diff/


Testing
---

Seems to work as expected, I will be attaching screenshots.


File Attachments (updated)


Showing events
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/c4c99bbd-f77f-4167-922e-79f90949040b__events.png
Showing photos from an event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/a4ed7663-64b6-4963-aed3-5e41cc8e51ee__showing-event.png
Editing dates of an existing event
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/07/12/965b667b-e209-44a3-bc64-95f81b18913e__editing-event.png


Thanks,

Shantanu Tushar

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


Re: Review Request 119219: Control volume from HomeScreen

2014-07-11 Thread Shantanu Tushar


 On July 10, 2014, 6:36 p.m., Shantanu Tushar wrote:
  My only problem with this is that its inconsistent with the Up/Down arrow 
  key combination that we have in MediaPlayer. Also, you can't make the 
  player also use Ctrl+Arrow keys because then a remote control won't work.
  I'd suggest just making sure that the standard media keys work (play,pause 
  etc) so that people can, for example, pause stuff when they get a phone 
  call. To control the volume they can always press Esc and Up/Down.
 
 R.Harish  Navnit wrote:
 Why is it inconsistent ? I mean I'm able to use both(just the arrow keys 
 and the combination). I've no idea of how a remote control would affect 
 things so I don't know what that would require. 
 I think the standard media keys do work. Yes, you can play/pause, no 
 problems there. but are you suggesting that I use the combination Esc + 
 Up/Down ? Well, I think that's inconsistent since the Esc key is being used 
 as a shortcut to go to the previous screen. However, we do have the backspace 
 key doing the same. I could change that if that's what you mean ? 
 
 P.S: I'm not sure if I've got what you're trying to convey. Is what I 
 mentioned above right ?
 
 Shantanu Tushar wrote:
 Its inconsistent because the player needs only Up/Down (try it) but this 
 one needs Ctrl as well (ideally you'd want the same shortcuts everywhere). 
 Remote control concept is simple, every action should be possible use arrow 
 keys, enter, esc. I meant you can press Esc from the home screen to go to the 
 player, and then Up/Down to control the volume.
 
 R.Harish  Navnit wrote:
 Yes, the player volume can be controlled using the arrow keys, but they 
 won't work when you're not in the player screen. The user can get away if 
 he's in the home screen, but if the user was, let's say browsing through his 
 pictures collection then pressing the Esc key is not really a good choice(3/4 
 presses of Esc key would take you to the player and only then would pressing 
 the arrow key work), isn't that inconvenient ? 
 
 We can have the same shortcuts everywhere(merely arrows won't work, since 
 they're used for navigation in the homescreen etc), we could have a 
 combination for the same, if not ctrl + arrows, how about using shift ? Do 
 you have any suggestions ?

Yep, those are the media volume up/down keys (assuming your keyboard has them).


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119219/#review62090
---


On July 10, 2014, 6:16 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119219/
 ---
 
 (Updated July 10, 2014, 6:16 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 AFAIK, volume of a playing media cannot be controlled from the HomeScreen. 
 This patch enables increasing and decreasing the volume using the combination 
 of Ctrl + Up/Down keys.
 
 
 Diffs
 -
 
   shells/newshell/package/contents/ui/mediacenter.qml 
 15c9cee7f86573939ac92872fb2201d944e0589f 
 
 Diff: https://git.reviewboard.kde.org/r/119219/diff/
 
 
 Testing
 ---
 
 Use the ctrl + up/down keys to increase/decrease the volume of the current 
 playing media. seems to work fine. 
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: cmake does not work

2014-07-10 Thread Shantanu Tushar Jha
Hi Devanshu,

What distro are you using? Also, how have you installed Qt?


On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain devanshu.jain...@gmail.com
wrote:

 Hi Harish,
 Thanks for pastebin suggestion. I would keep it in mind next time.
 As for the typos: No I didn't make any typos. I double checked everything.



 On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com
 wrote:

 On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Please find the enclosed cmake output log file


  Hi Devanshu,

 I would suggest using pastebin http://paste.kde.org instead of
 attaching the output logs in your mails. I think that's a lot easier and
 attachments in mails sent to ML's is not a good practice either.

 I don't think there's any hard and fast rule as such, but usually clones
 of projects that you wish to build and run are placed in the home dir, no ?
 (you may ignore this) and I hope you didn't make a typo while running the
 command that Sinny mentioned.

 Regards.
 R.Harish Navnit
 The Enigma http://harishnavnit.wordpress.com

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



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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: cmake does not work

2014-07-10 Thread Shantanu Tushar Jha
Cool, though I had asked how have you installed Qt ;) Because I've never
encountered this error.

Anyway, you can even add the fPIC flag like this -

cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config
--prefix`  -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4  -DKDE4_BUILD_TESTS=OFF



On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain devanshu.jain...@gmail.com
wrote:

 Hi Shantanu,
 I am using Ubuntu 14.04 and yes I have Qt installed.


 On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha shant...@kde.org
 wrote:

 Hi Devanshu,

 What distro are you using? Also, how have you installed Qt?


 On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Harish,
 Thanks for pastebin suggestion. I would keep it in mind next time.
 As for the typos: No I didn't make any typos. I double checked
 everything.



 On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit harishnav...@gmail.com
  wrote:

 On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Please find the enclosed cmake output log file


  Hi Devanshu,

 I would suggest using pastebin http://paste.kde.org instead of
 attaching the output logs in your mails. I think that's a lot easier and
 attachments in mails sent to ML's is not a good practice either.

 I don't think there's any hard and fast rule as such, but usually
 clones of projects that you wish to build and run are placed in the home
 dir, no ? (you may ignore this) and I hope you didn't make a typo while
 running the command that Sinny mentioned.

 Regards.
 R.Harish Navnit
 The Enigma http://harishnavnit.wordpress.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: cmake does not work

2014-07-10 Thread Shantanu Tushar Jha
try running this and pastebin the output -

VERBOSE=1 make



On Thu, Jul 10, 2014 at 9:57 PM, Devanshu Jain devanshu.jain...@gmail.com
wrote:

 I started from scratch but make command is again giving the same error.


 On Thu, Jul 10, 2014 at 9:45 PM, Shantanu Tushar Jha shant...@kde.org
 wrote:

 Something tells me its still using Qt5 (and thats why it regards 4.8.6 as
 old), can you try removing your build dir and start from scratch?

 About Qt, do *not* use the one downloaded from the website, the one that
 you install using apt-get/synaptic is the way to go.


 On Thu, Jul 10, 2014 at 9:27 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Shantanu,
 Sorry for misreading.
 But I downloaded Qt using synaptic. And I guess it is still causing
 troubles.
 When doing *make install*, another error is produced (
 http://paste.kde.org/prtoragcp).
 It says:
 This file was generated using the moc from 4.8.6. It cannot be used with
 the include files from this version of Qt. (The moc has changed too much.)
 So I guess I will download it again from the official site.


 On Thu, Jul 10, 2014 at 9:15 PM, Shantanu Tushar Jha shaan...@gmail.com
  wrote:

 Cool, though I had asked how have you installed Qt ;) Because I've
 never encountered this error.

 Anyway, you can even add the fPIC flag like this -

 cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config
 --prefix`  -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4  -DKDE4_BUILD_TESTS=OFF



 On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Shantanu,
 I am using Ubuntu 14.04 and yes I have Qt installed.


 On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha shant...@kde.org
  wrote:

 Hi Devanshu,

 What distro are you using? Also, how have you installed Qt?


 On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Harish,
 Thanks for pastebin suggestion. I would keep it in mind next time.
 As for the typos: No I didn't make any typos. I double checked
 everything.



 On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit 
 harishnav...@gmail.com wrote:

 On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Please find the enclosed cmake output log file


  Hi Devanshu,

 I would suggest using pastebin http://paste.kde.org instead of
 attaching the output logs in your mails. I think that's a lot easier 
 and
 attachments in mails sent to ML's is not a good practice either.

 I don't think there's any hard and fast rule as such, but usually
 clones of projects that you wish to build and run are placed in the 
 home
 dir, no ? (you may ignore this) and I hope you didn't make a typo while
 running the command that Sinny mentioned.

 Regards.
 R.Harish Navnit
 The Enigma http://harishnavnit.wordpress.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: cmake does not work

2014-07-10 Thread Shantanu Tushar Jha
Hmm that is very strange. Edit the file
build/libs/mediacenter/moc_mediasourcesloader.cpp and put these just after
the != 63 condition-

#error VERSION IS
#error Q_MOC_OUTPUT_REVISION

and then run make again and pastebin



On Thu, Jul 10, 2014 at 10:35 PM, Devanshu Jain devanshu.jain...@gmail.com
wrote:

 Here is the link to the output: http://paste.kde.org/pjyrnmzag


 On Thu, Jul 10, 2014 at 10:33 PM, Shantanu Tushar Jha shant...@kde.org
 wrote:

 try running this and pastebin the output -

 VERBOSE=1 make



 On Thu, Jul 10, 2014 at 9:57 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 I started from scratch but make command is again giving the same error.


 On Thu, Jul 10, 2014 at 9:45 PM, Shantanu Tushar Jha shant...@kde.org
 wrote:

 Something tells me its still using Qt5 (and thats why it regards 4.8.6
 as old), can you try removing your build dir and start from scratch?

 About Qt, do *not* use the one downloaded from the website, the one
 that you install using apt-get/synaptic is the way to go.


 On Thu, Jul 10, 2014 at 9:27 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Shantanu,
 Sorry for misreading.
 But I downloaded Qt using synaptic. And I guess it is still causing
 troubles.
 When doing *make install*, another error is produced (
 http://paste.kde.org/prtoragcp).
 It says:
 This file was generated using the moc from 4.8.6. It cannot be used
 with the include files from this version of Qt. (The moc has changed too
 much.)
 So I guess I will download it again from the official site.


 On Thu, Jul 10, 2014 at 9:15 PM, Shantanu Tushar Jha 
 shaan...@gmail.com wrote:

 Cool, though I had asked how have you installed Qt ;) Because I've
 never encountered this error.

 Anyway, you can even add the fPIC flag like this -

 cmake .. -DCMAKE_CXX_FLAGS=-fPIC -DCMAKE_INSTALL_PREFIX=`kde4-config
 --prefix`  -DQT_QMAKE_EXECUTABLE=/usr/bin/qmake-qt4  
 -DKDE4_BUILD_TESTS=OFF



 On Thu, Jul 10, 2014 at 8:33 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Shantanu,
 I am using Ubuntu 14.04 and yes I have Qt installed.


 On Thu, Jul 10, 2014 at 8:26 PM, Shantanu Tushar Jha 
 shant...@kde.org wrote:

 Hi Devanshu,

 What distro are you using? Also, how have you installed Qt?


 On Thu, Jul 10, 2014 at 4:48 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Hi Harish,
 Thanks for pastebin suggestion. I would keep it in mind next time.
 As for the typos: No I didn't make any typos. I double checked
 everything.



 On Thu, Jul 10, 2014 at 4:37 PM, R.Harish Navnit 
 harishnav...@gmail.com wrote:

 On Thu, Jul 10, 2014 at 12:35 PM, Devanshu Jain 
 devanshu.jain...@gmail.com wrote:

 Please find the enclosed cmake output log file


  Hi Devanshu,

 I would suggest using pastebin http://paste.kde.org instead of
 attaching the output logs in your mails. I think that's a lot easier 
 and
 attachments in mails sent to ML's is not a good practice either.

 I don't think there's any hard and fast rule as such, but usually
 clones of projects that you wish to build and run are placed in the 
 home
 dir, no ? (you may ignore this) and I hope you didn't make a typo 
 while
 running the command that Sinny mentioned.

 Regards.
 R.Harish Navnit
 The Enigma http://harishnavnit.wordpress.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



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




 --
 Shantanu Tushar(UTC +0530)
 http://www.shantanutushar.com

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



 ___
 Plasma-devel mailing

Re: Review Request 119219: Control volume from HomeScreen

2014-07-10 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119219/#review62090
---


My only problem with this is that its inconsistent with the Up/Down arrow key 
combination that we have in MediaPlayer. Also, you can't make the player also 
use Ctrl+Arrow keys because then a remote control won't work.
I'd suggest just making sure that the standard media keys work (play,pause etc) 
so that people can, for example, pause stuff when they get a phone call. To 
control the volume they can always press Esc and Up/Down.

- Shantanu Tushar


On July 10, 2014, 6:16 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119219/
 ---
 
 (Updated July 10, 2014, 6:16 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 AFAIK, volume of a playing media cannot be controlled from the HomeScreen. 
 This patch enables increasing and decreasing the volume using the combination 
 of Ctrl + Up/Down keys.
 
 
 Diffs
 -
 
   shells/newshell/package/contents/ui/mediacenter.qml 
 15c9cee7f86573939ac92872fb2201d944e0589f 
 
 Diff: https://git.reviewboard.kde.org/r/119219/diff/
 
 
 Testing
 ---
 
 Use the ctrl + up/down keys to increase/decrease the volume of the current 
 playing media. seems to work fine. 
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 119219: Control volume from HomeScreen

2014-07-10 Thread Shantanu Tushar


 On July 10, 2014, 6:36 p.m., Shantanu Tushar wrote:
  My only problem with this is that its inconsistent with the Up/Down arrow 
  key combination that we have in MediaPlayer. Also, you can't make the 
  player also use Ctrl+Arrow keys because then a remote control won't work.
  I'd suggest just making sure that the standard media keys work (play,pause 
  etc) so that people can, for example, pause stuff when they get a phone 
  call. To control the volume they can always press Esc and Up/Down.
 
 R.Harish  Navnit wrote:
 Why is it inconsistent ? I mean I'm able to use both(just the arrow keys 
 and the combination). I've no idea of how a remote control would affect 
 things so I don't know what that would require. 
 I think the standard media keys do work. Yes, you can play/pause, no 
 problems there. but are you suggesting that I use the combination Esc + 
 Up/Down ? Well, I think that's inconsistent since the Esc key is being used 
 as a shortcut to go to the previous screen. However, we do have the backspace 
 key doing the same. I could change that if that's what you mean ? 
 
 P.S: I'm not sure if I've got what you're trying to convey. Is what I 
 mentioned above right ?

Its inconsistent because the player needs only Up/Down (try it) but this one 
needs Ctrl as well (ideally you'd want the same shortcuts everywhere). Remote 
control concept is simple, every action should be possible use arrow keys, 
enter, esc. I meant you can press Esc from the home screen to go to the player, 
and then Up/Down to control the volume.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119219/#review62090
---


On July 10, 2014, 6:16 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119219/
 ---
 
 (Updated July 10, 2014, 6:16 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 AFAIK, volume of a playing media cannot be controlled from the HomeScreen. 
 This patch enables increasing and decreasing the volume using the combination 
 of Ctrl + Up/Down keys.
 
 
 Diffs
 -
 
   shells/newshell/package/contents/ui/mediacenter.qml 
 15c9cee7f86573939ac92872fb2201d944e0589f 
 
 Diff: https://git.reviewboard.kde.org/r/119219/diff/
 
 
 Testing
 ---
 
 Use the ctrl + up/down keys to increase/decrease the volume of the current 
 playing media. seems to work fine. 
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 119015: Fixing full screen settings on Plasma Mediacenter exit

2014-07-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119015/#review61494
---

Ship it!


Looks to be good, just a small modification and this can go in.


browsingbackends/utility/exit/exitbackend.cpp
https://git.reviewboard.kde.org/r/119015/#comment42806

this should not be needed anymore


- Shantanu Tushar


On June 29, 2014, 3:53 p.m., Sujith Haridasan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119015/
 ---
 
 (Updated June 29, 2014, 3:53 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This patch fixes the full screen settings of plasma-mediacenter during exit. 
 Though its bit hackish which I had done.
 I would like to make this done better. With this change the pmc will save the 
 fullscreen settings before exit.
 
 
 Diffs
 -
 
   browsingbackends/utility/exit/CMakeLists.txt 48d12a7 
   browsingbackends/utility/exit/exitbackend.cpp 9fe6cad 
   shells/newshell/mainwindow.cpp 97655d8 
 
 Diff: https://git.reviewboard.kde.org/r/119015/diff/
 
 
 Testing
 ---
 
 Done testing with fullscreen enabled and exit  fullscreen disabled and 
 exit. Both results were as expected.
 
 
 Thanks,
 
 Sujith Haridasan
 


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


Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.

2014-06-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review60666
---


Looks good to me, just one suggestion


mediaelements/mediacontroller/MediaController.qml
https://git.reviewboard.kde.org/r/116874/#comment42305

runtimeData.playing || runtimeData.paused should work better, thats usually 
the case in other media players


- Shantanu Tushar


On June 19, 2014, 9:04 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116874/
 ---
 
 (Updated June 19, 2014, 9:04 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 330990 and Bug
 http://bugs.kde.org/show_bug.cgi?id=330990
 http://bugs.kde.org/show_bug.cgi?id=Bug
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Now the next and previous buttons do not work unless a media is playing. 
 
 
 Diffs
 -
 
   mediaelements/mediacontroller/MediaController.qml 2fce0a0 
 
 Diff: https://git.reviewboard.kde.org/r/116874/diff/
 
 
 Testing
 ---
 
 I've done the testing for this patch.
 
 These are the steps that I followed. 
 1. Added a new playlist and added a few songs to the playlist.
 2. Re-opened Plasma Media Center 
 3. Clicked the Next and Previous buttons.
 4. Nothing was played.
 5. Selected a song from the list and then tested the next and previous 
 buttons.
 6. The buttons worked seamlessly.  
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.

2014-06-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review60669
---

Ship it!


Ship It!

- Shantanu Tushar


On June 21, 2014, 3:55 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116874/
 ---
 
 (Updated June 21, 2014, 3:55 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 330990 and Bug
 http://bugs.kde.org/show_bug.cgi?id=330990
 http://bugs.kde.org/show_bug.cgi?id=Bug
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Now the next and previous buttons do not work unless a media is playing. 
 
 
 Diffs
 -
 
   mediaelements/mediacontroller/MediaController.qml 2fce0a0 
 
 Diff: https://git.reviewboard.kde.org/r/116874/diff/
 
 
 Testing
 ---
 
 I've done the testing for this patch.
 
 These are the steps that I followed. 
 1. Added a new playlist and added a few songs to the playlist.
 2. Re-opened Plasma Media Center 
 3. Clicked the Next and Previous buttons.
 4. Nothing was played.
 5. Selected a song from the list and then tested the next and previous 
 buttons.
 6. The buttons worked seamlessly.  
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Review Request 118866: Start playing after Play All is used

2014-06-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118866/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

We should ask the model to start playing once we add all the songs to the 
playlist as a result of the user using Play All.


Diffs
-

  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
68eecf3 

Diff: https://git.reviewboard.kde.org/r/118866/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 118866: Start playing after Play All is used

2014-06-21 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118866/
---

(Updated June 21, 2014, 5:43 p.m.)


Review request for Plasma.


Changes
---

switch to and clear the default playlist only


Repository: plasma-mediacenter


Description
---

We should ask the model to start playing once we add all the songs to the 
playlist as a result of the user using Play All.


Diffs (updated)
-

  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
68eecf3 
  libs/mediacenter/multipleplaylistmodel.h 1c7731b 
  libs/mediacenter/multipleplaylistmodel.cpp 4b475c7 
  libs/mediacenter/playlistmodel.h 50157ff 
  libs/mediacenter/playlistmodel.cpp 44abbc1 

Diff: https://git.reviewboard.kde.org/r/118866/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 118824: Fix for Bug 336414 - Playlist switching doesnt move highlight

2014-06-19 Thread Shantanu Tushar


 On June 19, 2014, 8:16 a.m., Shantanu Tushar wrote:
  Good analysis and fix. It is still broken if you do the following-
  1. Have multiple playlists
  2. Remove a playlist
  3. Add a playlist
  
  After this the highlight again stops to work, can you check?

Hmm I can no longer reproduce this, seems to work :/ Maybe I wasn't running the 
correct patched binary in a hurry at office.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118824/#review60467
---


On June 19, 2014, 6:51 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118824/
 ---
 
 (Updated June 19, 2014, 6:51 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 336414
 http://bugs.kde.org/show_bug.cgi?id=336414
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Whenever we added / removed any playlist, the highlight was stuck at one 
 point i.e changing the playlist after that was not changing the highlight.
 
 This was happening because we were directly assigning value to 
 multiplePlaylistList.currentIndex in qml code, which removed its previous 
 binding. Instead we should emit the currentIndexChanged() signal and let the 
 ListView do the rest.
 
 
 Diffs
 -
 
   libs/mediacenter/multipleplaylistmodel.cpp 87601f5 
   mediaelements/playlist/MultiplePlaylists.qml 4b085fe 
 
 Diff: https://git.reviewboard.kde.org/r/118824/diff/
 
 
 Testing
 ---
 
 Changed playlist after adding / removing a playlist. Now highlight is working 
 fine
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118824: Fix for Bug 336414 - Playlist switching doesnt move highlight

2014-06-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118824/#review60517
---

Ship it!


Ship It!

- Shantanu Tushar


On June 19, 2014, 6:51 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118824/
 ---
 
 (Updated June 19, 2014, 6:51 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 336414
 http://bugs.kde.org/show_bug.cgi?id=336414
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Whenever we added / removed any playlist, the highlight was stuck at one 
 point i.e changing the playlist after that was not changing the highlight.
 
 This was happening because we were directly assigning value to 
 multiplePlaylistList.currentIndex in qml code, which removed its previous 
 binding. Instead we should emit the currentIndexChanged() signal and let the 
 ListView do the rest.
 
 
 Diffs
 -
 
   libs/mediacenter/multipleplaylistmodel.cpp 87601f5 
   mediaelements/playlist/MultiplePlaylists.qml 4b085fe 
 
 Diff: https://git.reviewboard.kde.org/r/118824/diff/
 
 
 Testing
 ---
 
 Changed playlist after adding / removing a playlist. Now highlight is working 
 fine
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118767: Fix for Bug 328532 - Remember the playlist I was using when I closed PMC last time

2014-06-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118767/#review60541
---

Ship it!


Seems to work correctly, just a minor nitpick and this can go in.


libs/mediacenter/playlistmodel.cpp
https://git.reviewboard.kde.org/r/118767/#comment42270

The explicit call to QVariant should not be needed


- Shantanu Tushar


On June 19, 2014, 4:30 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118767/
 ---
 
 (Updated June 19, 2014, 4:30 p.m.)
 
 
 Review request for Plasma, Bhushan Shah, Nikolaos Chatzidakis, Shantanu 
 Tushar, Sinny Kumari, and Sujith Haridasan.
 
 
 Bugs: 328532
 http://bugs.kde.org/show_bug.cgi?id=328532
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 PMC didn't used to remember the playlist I was on the last time, when 
 restarting it.
 Now it saves the lastPlaylist while closing in the settings, and when 
 starting it loads the value of lastPlaylist in settings
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.cpp dccaf52 
 
 Diff: https://git.reviewboard.kde.org/r/118767/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118767: Fix for Bug 328532 - Remember the playlist I was using when I closed PMC last time

2014-06-19 Thread Shantanu Tushar Jha
Hi Harish,

This is a known issue and is due to the fact that we don't save state while
switching the playlists. Maybe someone should report a bug if not already.


On Thu, Jun 19, 2014 at 10:44 PM, R.Harish Navnit harishnav...@gmail.com
wrote:

This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118767/

 On June 19th, 2014, 4:48 p.m. UTC, *Shantanu Tushar* wrote:

   libs/mediacenter/playlistmodel.cpp
 https://git.reviewboard.kde.org/r/118767/diff/2/?file=282626#file282626line56
  (Diff
 revision 2)

 public:

56

 d-playlistName = s.value(lastPlaylist, 
 QVariant(DEFAULT_PLAYLIST_NAME)).toString();

   The explicit call to QVariant should not be needed

  I have just one issue in this. This is how I tested it.

 1. Play a media from a playlist
 2. Switch to another playlist
 3. Return back to the playlist playing the current song.
 4. Next and prev don't work.
 5. Once the track finshes playing, the next track is not played. The 
 mediacenter stops playing any media and I have to mannually choose a new 
 media again.

 Imagine doing this over and over again once playlists are switched.

 Does anyone else face the same issue ?


 - R.Harish

 On June 19th, 2014, 4:30 p.m. UTC, Ashish Madeti wrote:
   Review request for Plasma, Bhushan Shah, Nikolaos Chatzidakis, Shantanu
 Tushar, Sinny Kumari, and Sujith Haridasan.
 By Ashish Madeti.

 *Updated June 19, 2014, 4:30 p.m.*
  *Bugs: * 328532 http://bugs.kde.org/show_bug.cgi?id=328532
  *Repository: * plasma-mediacenter
 Description

 PMC didn't used to remember the playlist I was on the last time, when 
 restarting it.
 Now it saves the lastPlaylist while closing in the settings, and when 
 starting it loads the value of lastPlaylist in settings

   Diffs

- libs/mediacenter/playlistmodel.cpp (dccaf52)

 View Diff https://git.reviewboard.kde.org/r/118767/diff/




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Ideas around Highlight

2014-06-15 Thread Shantanu Tushar Jha
Hey folks,

I recently added a visual highlight in the All Music mode to help the user
follow the current focus[1].

I am wondering if the simple highlight looks better than
PlasmaComponents.Highlight, and I need your opinion. Please try the patch
at http://paste.ubuntu.com/7647243 and see which one you prefer.

For folks who don't have the code running, here's a before[2] and after[3].

Cheers!

[1]
commits.kde.org/plasma-mediacenter/1e77bc1e19fb9f59ebb82aad9e95ff7ad966f130
[2] http://i.imgur.com/DO5cCA3.jpg
[3] http://i.imgur.com/CNxPWiz.jpg

-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 118759: Add a separator to the path where PMC stores playlist file

2014-06-15 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118759/#review60125
---



libs/mediacenter/playlistmodel.cpp
https://git.reviewboard.kde.org/r/118759/#comment41879

I'd prefer
QDir(getPlaylistPath()).absoluteFilePath(d-playlistName)


- Shantanu Tushar


On June 15, 2014, 10:14 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118759/
 ---
 
 (Updated June 15, 2014, 10:14 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Due to a lack of separator, PMC was storing the file in a different directory 
 and reading playlists from a different directory.
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.cpp 4c52f00 
 
 Diff: https://git.reviewboard.kde.org/r/118759/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118759: Add a separator to the path where PMC stores playlist file

2014-06-15 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118759/#review60126
---

Ship it!


Ship It!

- Shantanu Tushar


On June 15, 2014, 12:30 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118759/
 ---
 
 (Updated June 15, 2014, 12:30 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Due to a lack of separator, PMC was storing the file in a different directory 
 and reading playlists from a different directory.
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.cpp 4c52f00 
 
 Diff: https://git.reviewboard.kde.org/r/118759/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118734: Added MPRIS specifications Tracklist Interface

2014-06-14 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118734/#review60061
---



libs/mediacenter/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/118734/#comment41814

{ } please



libs/mediacenter/mpris2/mediaplayer2tracklist.h
https://git.reviewboard.kde.org/r/118734/#comment41815

C++11 magic: you can just say-

int tidCounter = 0;



libs/mediacenter/mpris2/mediaplayer2tracklist.cpp
https://git.reviewboard.kde.org/r/118734/#comment41816

not needed (see above)


- Shantanu Tushar


On June 13, 2014, 5:13 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118734/
 ---
 
 (Updated June 13, 2014, 5:13 p.m.)
 
 
 Review request for Plasma, Emmanuel Pescosta, Shantanu Tushar, and Sinny 
 Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Added a MPRIS specifications Tracklist Interface, which will expose and give 
 control over the current playlist on dbus.
 http://specifications.freedesktop.org/mpris-spec/latest/Track_List_Interface.html
 
 
 Diffs
 -
 
   libs/mediacenter/CMakeLists.txt 8d95cfc 
   libs/mediacenter/mpris2/mediaplayer2player.h 7a2f583 
   libs/mediacenter/mpris2/mediaplayer2player.cpp 2de56c2 
   libs/mediacenter/mpris2/mediaplayer2tracklist.h PRE-CREATION 
   libs/mediacenter/mpris2/mediaplayer2tracklist.cpp PRE-CREATION 
   libs/mediacenter/mpris2/mpris2.h 1dd78ca 
   libs/mediacenter/mpris2/mpris2.cpp b64ba04 
   shells/newshell/mainwindow.cpp 3c888f8 
 
 Diff: https://git.reviewboard.kde.org/r/118734/diff/
 
 
 Testing
 ---
 
 Tested the properties using qdbusviewer.
 I created a qt console application for testing whether the signals are being 
 emitted correctly and the methods are working.
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118734: Added MPRIS specifications Tracklist Interface

2014-06-14 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118734/#review60073
---

Ship it!


Ship It!

- Shantanu Tushar


On June 14, 2014, 8:15 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118734/
 ---
 
 (Updated June 14, 2014, 8:15 a.m.)
 
 
 Review request for Plasma, Emmanuel Pescosta, Shantanu Tushar, and Sinny 
 Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Added a MPRIS specifications Tracklist Interface, which will expose and give 
 control over the current playlist on dbus.
 http://specifications.freedesktop.org/mpris-spec/latest/Track_List_Interface.html
 
 
 Diffs
 -
 
   libs/mediacenter/CMakeLists.txt 8d95cfc 
   libs/mediacenter/mpris2/mediaplayer2player.h 7a2f583 
   libs/mediacenter/mpris2/mediaplayer2player.cpp 2de56c2 
   libs/mediacenter/mpris2/mediaplayer2tracklist.h PRE-CREATION 
   libs/mediacenter/mpris2/mediaplayer2tracklist.cpp PRE-CREATION 
   libs/mediacenter/mpris2/mpris2.h 1dd78ca 
   libs/mediacenter/mpris2/mpris2.cpp b64ba04 
   shells/newshell/mainwindow.cpp 3c888f8 
 
 Diff: https://git.reviewboard.kde.org/r/118734/diff/
 
 
 Testing
 ---
 
 Tested the properties using qdbusviewer.
 I created a qt console application for testing whether the signals are being 
 emitted correctly and the methods are working.
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist

2014-06-13 Thread Shantanu Tushar


 On June 12, 2014, 5:54 p.m., Mark Gaiser wrote:
  Why would you do it this way?
  I mean, you should have a model in the C++ side and you should have a view 
  (probably ListView). Then you can just ask the view the count: 
  http://qt-project.org/doc/qt-5/qml-qtquick-listview.html#count-prop i kinda 
  miss the point of doing this via C++. But then again, i don't know the code 
  in this project.
 
 Ashish Madeti wrote:
 I don't understand how 'count' is relevant here (I didn't get your 
 point). We want to set the currentIndex (which points to the item currently 
 playing in playlist) to -1 whenever a user plays something from other than 
 playlist.
 
 Mark Gaiser wrote:
 Ahh, sorry. I was wrong about the count property. It has nothing to do 
 with this.
 But you could have used the currentIndex property: 
 http://qt-project.org/doc/qt-5/qml-qtquick-listview.html#currentIndex-prop
 But that would only work if you also use the highlight functionality in 
 the views. In this case there should be no item highlighted which means the 
 ListView currentIndex property is -1.
 
 Does it make a bit more sense now? :)

We do use the currentIndex property of the view, we have two things-
1. Currently selected item in the View (the one with highlight), the user can 
move this using arrow keys/mouse.
2. Currently *playing* item in the playlist.

For 1 we use currentIndex of the View and 2 is implemented using a currentIndex 
property in the model (could use a better name though)


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118691/#review59895
---


On June 12, 2014, 5:51 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118691/
 ---
 
 (Updated June 12, 2014, 5:51 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Change the currentIndex of playlistModel to -1 when a media is played from 
 somewhere other than playlist.
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.h 0de7c56 
   libs/mediacenter/playlistmodel.cpp c5ab1ab 
   shells/newshell/package/contents/ui/mediacenter.qml a859225 
 
 Diff: https://git.reviewboard.kde.org/r/118691/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Review Request 118697: Treat albums with different artists as different albums

2014-06-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118697/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

This one has been pending for some time now. Our Album data model assumed that 
a album name is enough to uniquely identify an album. This creates problems 
because multiple artists may have albums with the same name.
This patch makes Media Library and ItemCache aware of this fact.


Diffs
-

  libs/mediacenter/album.h 4926e48 
  libs/mediacenter/album.cpp 881f65a 
  libs/mediacenter/itemcache.h ffee05f 
  libs/mediacenter/medialibrary.cpp 9c58b44 
  libs/test/itemcachetest.h 4fc6807 
  libs/test/itemcachetest.cpp e6dee33 
  libs/test/medialibrarytest.h 718457d 
  libs/test/medialibrarytest.cpp e1a8c9b 
  libs/test/mediatest.cpp 8fde60b 
  libs/test/pmcalbumtest.cpp 9f2b4c8 

Diff: https://git.reviewboard.kde.org/r/118697/diff/


Testing
---

Unit tests pass. Added test to take care of the new scenario.


Thanks,

Shantanu Tushar

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


Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist

2014-06-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118691/#review59887
---



libs/mediacenter/playlistmodel.h
https://git.reviewboard.kde.org/r/118691/#comment41712

currentIndex is not writable from QML so that PlaylistModel can have 
complete control over it.
A better way is to create a slot called resetCurrentIndex in PlaylistModel 
which will set it to -1.


- Shantanu Tushar


On June 12, 2014, 8:58 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118691/
 ---
 
 (Updated June 12, 2014, 8:58 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Change the currentIndex of playlistModel to -1 when a media is played from 
 somewhere other than playlist.
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.h 0de7c56 
   shells/newshell/package/contents/ui/mediacenter.qml a859225 
 
 Diff: https://git.reviewboard.kde.org/r/118691/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118691: Change currentIndex of playlistModel to -1 when a media is not from playlist

2014-06-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118691/#review59891
---

Ship it!


Ship It!

- Shantanu Tushar


On June 12, 2014, 5:22 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118691/
 ---
 
 (Updated June 12, 2014, 5:22 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Change the currentIndex of playlistModel to -1 when a media is played from 
 somewhere other than playlist.
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.h 0de7c56 
   libs/mediacenter/playlistmodel.cpp c5ab1ab 
   shells/newshell/package/contents/ui/mediacenter.qml a859225 
 
 Diff: https://git.reviewboard.kde.org/r/118691/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118657: Use move() while moving items in playlist, instead of dataChanged() signals

2014-06-11 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118657/#review59816
---

Ship it!


Ship It!

- Shantanu Tushar


On June 11, 2014, 5:51 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118657/
 ---
 
 (Updated June 11, 2014, 5:51 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Use beginMoveRows() and endMoveRows() instead of dataChanged() while moving 
 items in playlistModel
 
 
 Diffs
 -
 
   libs/mediacenter/playlistmodel.cpp 158ca65 
 
 Diff: https://git.reviewboard.kde.org/r/118657/diff/
 
 
 Testing
 ---
 
 Moved media around in playlist, its working fine.
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.

2014-06-10 Thread Shantanu Tushar


 On June 5, 2014, 8:36 a.m., Thomas Pfeiffer wrote:
  Usability review:
  Since I lack the skills to picture it from the diff: When exactly is the 
  notification shown? Is it shown as soon as the media is supposed to be 
  played? If so, I think it could be done in a more subtle way: Grey out the 
  entry in the playlist and just skip over it to the next entry int he 
  playlist (like Amarok does) and show the message in a tooltip on mouseover 
  on a pointer device or on tap on a touch device.
  If it's already done in a similar way, then it's fine for me ;)
 
 R.Harish  Navnit wrote:
 I've added a screenshot which should help. This is a little different 
 from the way amarok deals with the same error. If the media is inaccessible, 
 the message is shown in the player screen. 
 Is it okay, If I go ahead in the way I am or should look to implement 
 something similar to what amarok does ??
 
 Thomas Pfeiffer wrote:
 Does PMC really have to be restarted to detect that the location is now 
 accessible? That's not really a nice situation.
 The whole message seems overly dramatic to me, especially in a playlist. 
 Imagine you have a playlist with 50 songs in it, and three of them are on a 
 location which isn't accessible. What would you prefer?
 a) Having to close PMC, make the location accessible and then restart it
 b) Just skip over those three songs and happily listen to the other 47
 
 If we're not in a playlist, the problem should rarely occur because the 
 browser only shows currently accessible media anyway, right?

 
 R.Harish  Navnit wrote:
 So, like I'd mentioned in the mail(sorry about that). 
 
 1.PMC need not be restarted to get to detect that the location is 
 accessible. I think the message might have been ambiguous. The user is 
 expected to mount   
   all drives containing media and then run the application again. 
 2. You can skip over to the next media in a playlist. That's how I 
 envisage the fix.
 
 Please let me know things have to be done differently.
 
 Thomas Pfeiffer wrote:
 The problem is that currently, the message is very scary. The skip 
 controls are still active, but the user is presented with that very prominent 
 error message telling her that she has to make sure that the file becomes 
 accessible and restart the application. Plus, the playlist won't continue 
 unless the user actively skips to the next entry, right?
 The way Amarok does it is way less scary and disruptive since it silently 
 skips. The only problem with Amarok's solution is that there is no way to 
 find out why it skipped. That's why I suggested to offer a tooltip so users 
 who wonder why a track was skipped can find out what the problem is. The 
 message showing up in the tooltip could be like the one you've proposed, 
 though I would change it to Could not open the file [path]. To play this 
 file, make sure it is accessible and restart Plasma Mediacenter

 
 Shantanu Tushar wrote:
 Bah, this skipped me as well. There is absolutely no need to restart the 
 app here, if the file is available, simply clicking play again should work.
 
 Also, after Thomas pointed it out, in a playlist it actually makes sense 
 to simply skip the missing media. However, if you play a single media 
 (without the playlist) then this message seems appropriate.
 @Thomas, does that sound right?
 
 Finally, I'd suggest and try again, insteasd of and restart the 
 application.
 
 R.Harish  Navnit wrote:
 I find another discrepancy here, simply play again doesn't work even if 
 the file is available(after initially being inaccessible). There is no need 
 to restart the app here, but the only way that I'm able to play the media is 
 by playing either the next or previous media and then returning to the 
 current media. Clicking play again doesn't seem to work for me. I don't find 
 this too much of an issue btw, just reported though. 
 
 From the discussions that we've had ^^, I'm inferring that it is expected 
 that this fix works similar to the way in which amarok handles the same 
 situation, and I shall proceed along the same lines.

Well then its a bug, ideally clicking play again should work. This review 
should only go in once that bug is fixed imo (because it should change please 
restart to please try again)

About the behavior, in my previous comment I have suggested that this error 
should only come when the played media is *not* in the current playlist. If it 
is in playlist, we should silently skip it.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118406/#review59281
---


On June 5, 2014, 8:47 a.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail

Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.

2014-06-05 Thread Shantanu Tushar


 On June 5, 2014, 8:36 a.m., Thomas Pfeiffer wrote:
  Usability review:
  Since I lack the skills to picture it from the diff: When exactly is the 
  notification shown? Is it shown as soon as the media is supposed to be 
  played? If so, I think it could be done in a more subtle way: Grey out the 
  entry in the playlist and just skip over it to the next entry int he 
  playlist (like Amarok does) and show the message in a tooltip on mouseover 
  on a pointer device or on tap on a touch device.
  If it's already done in a similar way, then it's fine for me ;)
 
 R.Harish  Navnit wrote:
 I've added a screenshot which should help. This is a little different 
 from the way amarok deals with the same error. If the media is inaccessible, 
 the message is shown in the player screen. 
 Is it okay, If I go ahead in the way I am or should look to implement 
 something similar to what amarok does ??
 
 Thomas Pfeiffer wrote:
 Does PMC really have to be restarted to detect that the location is now 
 accessible? That's not really a nice situation.
 The whole message seems overly dramatic to me, especially in a playlist. 
 Imagine you have a playlist with 50 songs in it, and three of them are on a 
 location which isn't accessible. What would you prefer?
 a) Having to close PMC, make the location accessible and then restart it
 b) Just skip over those three songs and happily listen to the other 47
 
 If we're not in a playlist, the problem should rarely occur because the 
 browser only shows currently accessible media anyway, right?

 
 R.Harish  Navnit wrote:
 So, like I'd mentioned in the mail(sorry about that). 
 
 1.PMC need not be restarted to get to detect that the location is 
 accessible. I think the message might have been ambiguous. The user is 
 expected to mount   
   all drives containing media and then run the application again. 
 2. You can skip over to the next media in a playlist. That's how I 
 envisage the fix.
 
 Please let me know things have to be done differently.
 
 Thomas Pfeiffer wrote:
 The problem is that currently, the message is very scary. The skip 
 controls are still active, but the user is presented with that very prominent 
 error message telling her that she has to make sure that the file becomes 
 accessible and restart the application. Plus, the playlist won't continue 
 unless the user actively skips to the next entry, right?
 The way Amarok does it is way less scary and disruptive since it silently 
 skips. The only problem with Amarok's solution is that there is no way to 
 find out why it skipped. That's why I suggested to offer a tooltip so users 
 who wonder why a track was skipped can find out what the problem is. The 
 message showing up in the tooltip could be like the one you've proposed, 
 though I would change it to Could not open the file [path]. To play this 
 file, make sure it is accessible and restart Plasma Mediacenter


Bah, this skipped me as well. There is absolutely no need to restart the app 
here, if the file is available, simply clicking play again should work.

Also, after Thomas pointed it out, in a playlist it actually makes sense to 
simply skip the missing media. However, if you play a single media (without the 
playlist) then this message seems appropriate.
@Thomas, does that sound right?

Finally, I'd suggest and try again, insteasd of and restart the application.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118406/#review59281
---


On June 5, 2014, 8:47 a.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118406/
 ---
 
 (Updated June 5, 2014, 8:47 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 333764
 http://bugs.kde.org/show_bug.cgi?id=333764
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 If a media(in a playlist) is located in an inaccessible location, then the 
 user is notified about the same. 
 
 
 Diffs
 -
 
   mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c 
 
 Diff: https://git.reviewboard.kde.org/r/118406/diff/
 
 
 Testing
 ---
 
 1. Load media to a playlist.
 2. Unmount the device containing media.
 3. Check if the user is notified of the location being inaccessible
--yes, the user is notified
 4. Mount the device containing media and play a media from playlist.
-- The media plays properly.
 
 
 File Attachments
 
 
 wihtout_i18n.png
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/06/05/3dc148a5-c4da-4d27-a713-e63922cbcef8__wihtout_i18n.png
 
 
 Thanks,
 
 R.Harish  Navnit

Re: Review Request 118406: Notify the user if the location containing the media is inaccessible.

2014-06-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118406/#review59209
---



mediaelements/mediaplayer/MediaPlayer.qml
https://git.reviewboard.kde.org/r/118406/#comment41223

Remove the trailing space in the i18n call (after location).

Also, the i18n on video.source is of no use, people cant translate 
filenames, can they? :P



mediaelements/mediaplayer/MediaPlayer.qml
https://git.reviewboard.kde.org/r/118406/#comment41222

instead of guessing, why not use the error property of QML Video? 
http://doc.qt.digia.com/qtmobility-1.2.0/qml-video.html#error-prop


- Shantanu Tushar


On June 3, 2014, 5:05 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118406/
 ---
 
 (Updated June 3, 2014, 5:05 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 333764
 http://bugs.kde.org/show_bug.cgi?id=333764
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 If a media(in a playlist) is located in an inaccessible location, then the 
 user is notified about the same. 
 
 
 Diffs
 -
 
   mediaelements/mediaplayer/MediaPlayer.qml 98f1d2c 
 
 Diff: https://git.reviewboard.kde.org/r/118406/diff/
 
 
 Testing
 ---
 
 1. Load media to a playlist.
 2. Unmount the device containing media.
 3. Check if the user is notified of the location being inaccessible
--yes, the user is notified
 4. Mount the device containing media and play a media from playlist.
-- The media plays properly.
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 118468: Use the spacebar key to play/pause a media from the homescreen.

2014-06-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118468/#review59005
---

Ship it!


Hmm, looks good to me.

- Shantanu Tushar


On June 2, 2014, 12:24 p.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118468/
 ---
 
 (Updated June 2, 2014, 12:24 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 As of now, PMC only uses the MediaPlay button to give the user an option to 
 play/pause media from the home screen. However, not all keyboards have the 
 MediaPlay button AFAIK. 
 This patch gives the user the option to play/pause the media from the home 
 screen using the spacebar key as another alternative. 
 
 
 Diffs
 -
 
   shells/newshell/package/contents/ui/mediacenter.qml e1bc0e6 
 
 Diff: https://git.reviewboard.kde.org/r/118468/diff/
 
 
 Testing
 ---
 
 Play any media, navigate to homescreen. Press the spacebar to check if the 
 media plays/pauses. 
 
 Seems to work fine to me. 
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 118092: Left and right movement in All music

2014-05-17 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118092/#review58089
---

Ship it!


Ship It!

- Shantanu Tushar


On May 16, 2014, 6:36 p.m., Sujith Haridasan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118092/
 ---
 
 (Updated May 16, 2014, 6:36 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 334148
 http://bugs.kde.org/show_bug.cgi?id=334148
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Left and right movement in All Music - Songs section. Now user can navigate 
 left and right when in the songs section. And this is applicable only to the 
 songs section.
 
 
 Diffs
 -
 
   components/listbrowser/ListBrowser.qml 202d406 
 
 Diff: https://git.reviewboard.kde.org/r/118092/diff/
 
 
 Testing
 ---
 
 1) Use down arrow key to reach songs.
 2) Press left/right arrow key.
 3) Press left/right arrow key(for some reason we have to press the same key 
 twice).
 4) User reaches Artists if pressed left or Albums if pressed right.
 
 
 Thanks,
 
 Sujith Haridasan
 


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


Re: Review Request 118092: Left and right movement in All music

2014-05-16 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118092/#review58065
---



components/listbrowser/ListBrowser.qml
https://git.reviewboard.kde.org/r/118092/#comment40377

A ListBrowser does not need to be in a TabBar, (even though in All Music it 
is). So as a safeguard, do a check that listBrowserRoot.topSibling.buttons 
should be defined. Something like-
var buttons = listBrowserRoot.topSibling.buttons;
if (buttons) {
buttons.get(1).button.clicked();
}



- Shantanu Tushar


On May 16, 2014, 3:18 a.m., Sujith Haridasan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118092/
 ---
 
 (Updated May 16, 2014, 3:18 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 334148
 http://bugs.kde.org/show_bug.cgi?id=334148
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Left and right movement in All Music - Songs section. Now user can navigate 
 left and right when in the songs section. And this is applicable only to the 
 songs section.
 
 
 Diffs
 -
 
   components/listbrowser/ListBrowser.qml 202d406 
 
 Diff: https://git.reviewboard.kde.org/r/118092/diff/
 
 
 Testing
 ---
 
 1) Use down arrow key to reach songs.
 2) Press left/right arrow key.
 3) Press left/right arrow key(for some reason we have to press the same key 
 twice).
 4) User reaches Artists if pressed left or Albums if pressed right.
 
 
 Thanks,
 
 Sujith Haridasan
 


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


Re: Review Request 118092: Left and right movement in All music

2014-05-16 Thread Shantanu Tushar


 On May 16, 2014, 4:20 a.m., Ashish Madeti wrote:
  Now if you are on Songs tab, a single left/right key press will take you to 
  Artists/Albums section but when you return to Songs section, the focus is 
  still on tab bar. Due to this user has to first go to the list by pressing 
  down key once (not intuitive) whereas imho user should be able to continue 
  from where he left.

The bigger problem is that our up/down navigation is already not so clear in 
All Music. Fixing that should be better, i.e. to properly show that the focus 
is still on the TabBar. However, this is not in scope of this patch.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118092/#review58040
---


On May 16, 2014, 3:18 a.m., Sujith Haridasan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118092/
 ---
 
 (Updated May 16, 2014, 3:18 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 334148
 http://bugs.kde.org/show_bug.cgi?id=334148
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Left and right movement in All Music - Songs section. Now user can navigate 
 left and right when in the songs section. And this is applicable only to the 
 songs section.
 
 
 Diffs
 -
 
   components/listbrowser/ListBrowser.qml 202d406 
 
 Diff: https://git.reviewboard.kde.org/r/118092/diff/
 
 
 Testing
 ---
 
 1) Use down arrow key to reach songs.
 2) Press left/right arrow key.
 3) Press left/right arrow key(for some reason we have to press the same key 
 twice).
 4) User reaches Artists if pressed left or Albums if pressed right.
 
 
 Thanks,
 
 Sujith Haridasan
 


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


Re: Review Request 117984: Try to prioritize photos taken by a camera-like device

2014-05-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117984/
---

(Updated May 12, 2014, 2:14 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

The list of images that we get from Baloo has lot of noise because it usually 
contains small images from data files of software, games and so on.
We do two things-
* Ignore images unless they are wider than 500 pixels
* Make images with EXIF data appear first by giving them preference in sorting 
by date/time


Diffs
-

  plugins/baloosearch/audiosearchresulthandler.h 11b7219 
  plugins/baloosearch/audiosearchresulthandler.cpp de98bb7 
  plugins/baloosearch/baloosearchmediasource.h 93e3d80 
  plugins/baloosearch/baloosearchmediasource.cpp 4180b1e 
  plugins/baloosearch/imagesearchresulthandler.h 200451e 
  plugins/baloosearch/imagesearchresulthandler.cpp 272b476 
  plugins/baloosearch/searchresulthandler.h 0df0ba2 
  plugins/baloosearch/searchresulthandler.cpp 78e41ea 
  plugins/baloosearch/videosearchresulthandler.h 146efca 
  plugins/baloosearch/videosearchresulthandler.cpp 1295b30 

Diff: https://git.reviewboard.kde.org/r/117984/diff/


Testing
---

Photos are fetched, the ones from my recent trip to Kashmir show up first ;)


Thanks,

Shantanu Tushar

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


Re: Review Request 118092: Left and right movement in All music

2014-05-12 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118092/#review57797
---



components/listbrowser/ListBrowser.qml
https://git.reviewboard.kde.org/r/118092/#comment40236

As this only puts focus on the tab bar, you have to press left/right twice 
to make it work. Instead one press of the key should work.


- Shantanu Tushar


On May 12, 2014, 4:29 a.m., Sujith Haridasan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/118092/
 ---
 
 (Updated May 12, 2014, 4:29 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 334148
 http://bugs.kde.org/show_bug.cgi?id=334148
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Left and right movement in All Music - Songs section. Now user can navigate 
 left and right when in the songs section. And this is applicable only to the 
 songs section.
 
 
 Diffs
 -
 
   components/listbrowser/ListBrowser.qml 202d406 
 
 Diff: https://git.reviewboard.kde.org/r/118092/diff/
 
 
 Testing
 ---
 
 1) Use down arrow key to reach songs.
 2) Press left/right arrow key.
 3) Press left/right arrow key(for some reason we have to press the same key 
 twice).
 4) User reaches Artists if pressed left or Albums if pressed right.
 
 
 Thanks,
 
 Sujith Haridasan
 


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


Re: Review Request 118022: Replace hacky access to the declarative engine by a PmcRuntime class

2014-05-07 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118022/
---

(Updated May 7, 2014, 4:05 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

When backends needed to access some PMC objects like the playlist model or 
register image providers, they used to fetch a reference to the view's 
declarative engine. This is risky because it exposes the view to the backends 
which can then do anything with it.
This patch adds a PmcRuntime class which serves as an official interface to 
these accesses cleanly.


Diffs
-

  browsingbackends/localfiles/localfilesabstractbackend.h 2d47eba 
  browsingbackends/localfiles/localfilesabstractbackend.cpp b86a28a 
  browsingbackends/localfiles/localpictures/localpicturesbackend.cpp f025c04 
  browsingbackends/localfiles/localpictures/localpicturesmodel.h b1a81b4 
  browsingbackends/localfiles/localpictures/localpicturesmodel.cpp c6b1816 
  browsingbackends/localfiles/localvideos/localvideosbackend.cpp 8c20f27 
  browsingbackends/localfiles/localvideos/localvideosmodel.h e836c14 
  browsingbackends/localfiles/localvideos/localvideosmodel.cpp ab24013 
  browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.h 
50af6c2 
  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
6991c97 
  libs/mediacenter/CMakeLists.txt f07d3ec 
  libs/mediacenter/abstractbrowsingbackend.h 537d00d 
  libs/mediacenter/abstractbrowsingbackend.cpp eea4ee5 
  libs/mediacenter/backendsmodel.h f98f60a 
  libs/mediacenter/backendsmodel.cpp 3a5fc30 
  libs/mediacenter/pmcruntime.h PRE-CREATION 
  libs/mediacenter/pmcruntime.cpp PRE-CREATION 
  shells/newshell/mainwindow.h 0aaa513 
  shells/newshell/mainwindow.cpp e369055 
  shells/newshell/package/contents/ui/mediacenter.qml a62becc 

Diff: https://git.reviewboard.kde.org/r/118022/diff/


Testing
---

Adding songs to playlist, local pictures previews, local videos previews etc 
work fine.


Thanks,

Shantanu Tushar

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


Re: Review Request 118022: Replace hacky access to the declarative engine by a PmcRuntime class

2014-05-06 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118022/
---

(Updated May 6, 2014, 3:56 p.m.)


Review request for Plasma.


Changes
---

Removed FIXME and obsolete PmcRuntime from mediacenter.qml


Repository: plasma-mediacenter


Description
---

When backends needed to access some PMC objects like the playlist model or 
register image providers, they used to fetch a reference to the view's 
declarative engine. This is risky because it exposes the view to the backends 
which can then do anything with it.
This patch adds a PmcRuntime class which serves as an official interface to 
these accesses cleanly.


Diffs (updated)
-

  browsingbackends/localfiles/localfilesabstractbackend.h 2d47eba 
  browsingbackends/localfiles/localfilesabstractbackend.cpp b86a28a 
  browsingbackends/localfiles/localpictures/localpicturesbackend.cpp f025c04 
  browsingbackends/localfiles/localpictures/localpicturesmodel.h b1a81b4 
  browsingbackends/localfiles/localpictures/localpicturesmodel.cpp c6b1816 
  browsingbackends/localfiles/localvideos/localvideosbackend.cpp 8c20f27 
  browsingbackends/localfiles/localvideos/localvideosmodel.h e836c14 
  browsingbackends/localfiles/localvideos/localvideosmodel.cpp ab24013 
  browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.h 
50af6c2 
  
browsingbackends/metadatabackends/metadatamusicbackend/metadatamusicbackend.cpp 
6991c97 
  libs/mediacenter/CMakeLists.txt f07d3ec 
  libs/mediacenter/abstractbrowsingbackend.h 537d00d 
  libs/mediacenter/abstractbrowsingbackend.cpp eea4ee5 
  libs/mediacenter/backendsmodel.h f98f60a 
  libs/mediacenter/backendsmodel.cpp 3a5fc30 
  libs/mediacenter/pmcruntime.h PRE-CREATION 
  libs/mediacenter/pmcruntime.cpp PRE-CREATION 
  shells/newshell/mainwindow.h 0aaa513 
  shells/newshell/mainwindow.cpp e369055 
  shells/newshell/package/contents/ui/mediacenter.qml a62becc 

Diff: https://git.reviewboard.kde.org/r/118022/diff/


Testing
---

Adding songs to playlist, local pictures previews, local videos previews etc 
work fine.


Thanks,

Shantanu Tushar

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


Re: Plasma 5?

2014-05-05 Thread Shantanu Tushar Jha
+1

And, its easier to say as well :)


On Mon, May 5, 2014 at 8:51 PM, Marco Martin notm...@gmail.com wrote:

 On Monday 05 May 2014, Sebastian Kügler wrote:

  The baseline, to use Plasma as the brand, and only refer to the version
  as a technicality should of course stay the same.
 
  Why do I think 5 is better than 2014.6, or year.month of release?

 After trying (with not much success) to use publicly the next/2014.06
 scheme,
 I have to agree that I would feel more confortable with either Plasma 5 or
 Plasma 2
 To me either of which wouldn't make much difference, it's true tough that
 looks less confusing if it has the same number as frameworks and Qt


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




-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 117981: Filter out images with image width lesser than 500 pixel in Plasma media Center

2014-05-04 Thread Shantanu Tushar


 On May 4, 2014, 5:40 a.m., Bhushan Shah wrote:
  Same should be applied to baloo mediasource. Nepomuk mediasource is kind of 
  deprecated.

I'm working on it and will be submitting a patch soon.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117981/#review57225
---


On May 4, 2014, 5:22 a.m., Sinny Kumari wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117981/
 ---
 
 (Updated May 4, 2014, 5:22 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 When All Pictures gets opened within PMC, it shows all images indexed by 
 nepomuk. Sometime many images appear which are very small in size and are not 
 useful because those images come from some applications, system icons 
 etc.This patch will filter out images whose width is lesser than 500 pixel.
 
 
 Diffs
 -
 
   plugins/kdedesktopsearch/CMakeLists.txt ece8ef2 
   plugins/kdedesktopsearch/kdemetadatamediasource.h 2d8e7e2 
   plugins/kdedesktopsearch/kdemetadatamediasource.cpp 94c390a 
 
 Diff: https://git.reviewboard.kde.org/r/117981/diff/
 
 
 Testing
 ---
 
 Works as expected
 
 
 Thanks,
 
 Sinny Kumari
 


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


Review Request 117983: Nuke PmcImageProvider and streamline image providers

2014-05-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117983/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

The code around fetching/storing covers and other such images from remote 
sources and local files had become quite confusing over time. This patch 
removes unnecessary things and moves methods to the right places.


Diffs
-

  libs/mediacenter/CMakeLists.txt 1072799 
  libs/mediacenter/pmccoverartprovider.h a4aeb83 
  libs/mediacenter/pmccoverartprovider.cpp c68afa2 
  libs/mediacenter/pmcimagecache.h 631d5bd 
  libs/mediacenter/pmcimagecache.cpp d1b57b5 
  libs/mediacenter/pmcimageprovider.h e584810 
  libs/mediacenter/pmcimageprovider.cpp 4198f56 
  libs/mediacenter/pmcmetadatamodel.cpp a58a57b 
  libs/test/lastfmimagefetchertest.cpp b0fcf6f 
  mediaelements/mediaplayer/MusicStats.qml 178a37d 
  mediaelements/popupmenu/PopupMenu.qml 1c87926 
  shells/newshell/mainwindow.cpp 764b228 

Diff: https://git.reviewboard.kde.org/r/117983/diff/


Testing
---

Tested the following things-
* Album and Artist cover art
* Cover in MusicStats.qml when playing from a file
* Thumbnails in All Videos


Thanks,

Shantanu Tushar

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


Review Request 117984: Try to prioritize photos taken by a camera-like device

2014-05-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117984/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

The list of images that we get from Baloo has lot of noise because it usually 
contains small images from data files of software, games and so on.
We do two things-
* Ignore images unless they are wider than 500 pixels
* Make images with EXIF data appear first by giving them preference in sorting 
by date/time


Diffs
-

  plugins/baloosearch/audiosearchresulthandler.h 11b7219 
  plugins/baloosearch/audiosearchresulthandler.cpp de98bb7 
  plugins/baloosearch/baloosearchmediasource.h 93e3d80 
  plugins/baloosearch/baloosearchmediasource.cpp 4180b1e 
  plugins/baloosearch/imagesearchresulthandler.h 200451e 
  plugins/baloosearch/imagesearchresulthandler.cpp 272b476 
  plugins/baloosearch/searchresulthandler.h 0df0ba2 
  plugins/baloosearch/searchresulthandler.cpp 78e41ea 
  plugins/baloosearch/videosearchresulthandler.h 146efca 
  plugins/baloosearch/videosearchresulthandler.cpp 1295b30 

Diff: https://git.reviewboard.kde.org/r/117984/diff/


Testing
---

Photos are fetched, the ones from my recent trip to Kashmir show up first ;)


Thanks,

Shantanu Tushar

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


Re: Review Request 117981: Filter out images with image width lesser than 500 pixel in Plasma media Center

2014-05-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117981/#review57227
---

Ship it!


Few nitpicks, looks okay otherwise.


plugins/kdedesktopsearch/kdemetadatamediasource.cpp
https://git.reviewboard.kde.org/r/117981/#comment39881

const QUrl



plugins/kdedesktopsearch/kdemetadatamediasource.cpp
https://git.reviewboard.kde.org/r/117981/#comment39880

Leftover?


- Shantanu Tushar


On May 4, 2014, 5:22 a.m., Sinny Kumari wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117981/
 ---
 
 (Updated May 4, 2014, 5:22 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 When All Pictures gets opened within PMC, it shows all images indexed by 
 nepomuk. Sometime many images appear which are very small in size and are not 
 useful because those images come from some applications, system icons 
 etc.This patch will filter out images whose width is lesser than 500 pixel.
 
 
 Diffs
 -
 
   plugins/kdedesktopsearch/CMakeLists.txt ece8ef2 
   plugins/kdedesktopsearch/kdemetadatamediasource.h 2d8e7e2 
   plugins/kdedesktopsearch/kdemetadatamediasource.cpp 94c390a 
 
 Diff: https://git.reviewboard.kde.org/r/117981/diff/
 
 
 Testing
 ---
 
 Works as expected
 
 
 Thanks,
 
 Sinny Kumari
 


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


Re: Review Request 117983: Nuke PmcImageProvider and streamline image providers

2014-05-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117983/
---

(Updated May 4, 2014, 8:19 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

The code around fetching/storing covers and other such images from remote 
sources and local files had become quite confusing over time. This patch 
removes unnecessary things and moves methods to the right places.


Diffs
-

  libs/mediacenter/CMakeLists.txt 1072799 
  libs/mediacenter/pmccoverartprovider.h a4aeb83 
  libs/mediacenter/pmccoverartprovider.cpp c68afa2 
  libs/mediacenter/pmcimagecache.h 631d5bd 
  libs/mediacenter/pmcimagecache.cpp d1b57b5 
  libs/mediacenter/pmcimageprovider.h e584810 
  libs/mediacenter/pmcimageprovider.cpp 4198f56 
  libs/mediacenter/pmcmetadatamodel.cpp a58a57b 
  libs/test/lastfmimagefetchertest.cpp b0fcf6f 
  mediaelements/mediaplayer/MusicStats.qml 178a37d 
  mediaelements/popupmenu/PopupMenu.qml 1c87926 
  shells/newshell/mainwindow.cpp 764b228 

Diff: https://git.reviewboard.kde.org/r/117983/diff/


Testing
---

Tested the following things-
* Album and Artist cover art
* Cover in MusicStats.qml when playing from a file
* Thumbnails in All Videos


Thanks,

Shantanu Tushar

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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Shantanu Tushar


 On May 2, 2014, 9:07 a.m., Vishesh Handa wrote:
  Looks fine from a Baloo point of view. I cannot comment on the PMC parts, I 
  don't know the code.

That is cool :)


 On May 2, 2014, 9:07 a.m., Vishesh Handa wrote:
  plugins/baloosearch/searchresulthandler.cpp, line 36
  https://git.reviewboard.kde.org/r/117915/diff/1/?file=270496#file270496line36
 
  Please keep in mind that this is sync, and you'll be blocking. You want 
  to put it another thread via the Runnable if you want it to be async.

That is fine, the media sources are themselves running in a separate thread.


 On May 2, 2014, 9:07 a.m., Vishesh Handa wrote:
  plugins/baloosearch/audiosearchresulthandler.cpp, line 60
  https://git.reviewboard.kde.org/r/117915/diff/1/?file=270490#file270490line60
 
  Considering that you're checking if the title is empty, do you want to 
  do the same for the Artist and Album?

Usually things like empty Album and Artist are handled by the Media Library 
which shows then as Unknown so there is no need to check this here.

For the title, when we haven't fetched the File metadata, we would've put the 
filename (foo.mp3) as the title. This check is to make sure that we don't end 
up erasing foo.mp3 unless the title The Foo Song is available in Baloo::File.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57102
---


On May 1, 2014, 6:06 a.m., Shantanu Tushar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117915/
 ---
 
 (Updated May 1, 2014, 6:06 a.m.)
 
 
 Review request for Plasma and Vishesh Handa.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Just like the Nepomuk media source, the Baloo media source needs to provide 
 the date/time information for media. This is used to sort the media to show 
 the more recent media first.
 For images, the date/time when the photo was actually taken is used, and the 
 file creation date/time is used for other media.
 
 
 Diffs
 -
 
   plugins/baloosearch/CMakeLists.txt 1ff81fb 
   plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/baloosearchmediasource.h e315de4 
   plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 
   plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/searchresulthandler.h PRE-CREATION 
   plugins/baloosearch/searchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/videosearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/117915/diff/
 
 
 Testing
 ---
 
 Tested with all three types of media, works fine. Unit tests for the new code 
 to follow.
 
 
 Thanks,
 
 Shantanu Tushar
 


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


Re: Review Request 117931: Restrict sourceSize to screen size

2014-05-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117931/
---

(Updated May 2, 2014, 2:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Bugs: 334176
http://bugs.kde.org/show_bug.cgi?id=334176


Repository: plasma-mediacenter


Description
---

This helps by limiting the image source width so that it doesnt load full 
versions of images with resolutions more than the screen size.


Diffs
-

  mediaelements/imageviewer/ImageViewer.qml b182bf2 

Diff: https://git.reviewboard.kde.org/r/117931/diff/


Testing
---

works with both portrait and landscape


Thanks,

Shantanu Tushar

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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-02 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/
---

(Updated May 2, 2014, 2:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Vishesh Handa.


Repository: plasma-mediacenter


Description
---

Just like the Nepomuk media source, the Baloo media source needs to provide the 
date/time information for media. This is used to sort the media to show the 
more recent media first.
For images, the date/time when the photo was actually taken is used, and the 
file creation date/time is used for other media.


Diffs
-

  plugins/baloosearch/CMakeLists.txt 1ff81fb 
  plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/baloosearchmediasource.h e315de4 
  plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 
  plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/searchresulthandler.h PRE-CREATION 
  plugins/baloosearch/searchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/117915/diff/


Testing
---

Tested with all three types of media, works fine. Unit tests for the new code 
to follow.


Thanks,

Shantanu Tushar

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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-01 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/
---

(Updated May 1, 2014, 6:06 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Vishesh Handa.


Repository: plasma-mediacenter


Description
---

Just like the Nepomuk media source, the Baloo media source needs to provide the 
date/time information for media. This is used to sort the media to show the 
more recent media first.
For images, the date/time when the photo was actually taken is used, and the 
file creation date/time is used for other media.


Diffs
-

  plugins/baloosearch/CMakeLists.txt 1ff81fb 
  plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/baloosearchmediasource.h e315de4 
  plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 
  plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/searchresulthandler.h PRE-CREATION 
  plugins/baloosearch/searchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/117915/diff/


Testing
---

Tested with all three types of media, works fine. Unit tests for the new code 
to follow.


Thanks,

Shantanu Tushar

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


Re: Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-05-01 Thread Shantanu Tushar


 On May 1, 2014, 6:06 a.m., Commit Hook wrote:
  This review has been submitted with commit 
  121e51d74ff50a3644f275d5cfb571326c7a7286 by Shantanu Tushar to branch 
  master.

Had pushed this by mistake when trying to fix a bug, reverted it. The review is 
still open.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/#review57044
---


On May 1, 2014, 6:06 a.m., Shantanu Tushar wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117915/
 ---
 
 (Updated May 1, 2014, 6:06 a.m.)
 
 
 Review request for Plasma and Vishesh Handa.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Just like the Nepomuk media source, the Baloo media source needs to provide 
 the date/time information for media. This is used to sort the media to show 
 the more recent media first.
 For images, the date/time when the photo was actually taken is used, and the 
 file creation date/time is used for other media.
 
 
 Diffs
 -
 
   plugins/baloosearch/CMakeLists.txt 1ff81fb 
   plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/baloosearchmediasource.h e315de4 
   plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 
   plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/searchresulthandler.h PRE-CREATION 
   plugins/baloosearch/searchresulthandler.cpp PRE-CREATION 
   plugins/baloosearch/videosearchresulthandler.h PRE-CREATION 
   plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/117915/diff/
 
 
 Testing
 ---
 
 Tested with all three types of media, works fine. Unit tests for the new code 
 to follow.
 
 
 Thanks,
 
 Shantanu Tushar
 


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


Re: Review Request 117923: Export everything by defult for test in plasmamediacentertest librray

2014-05-01 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117923/#review57046
---

Ship it!


Cool, looks to do what is expected. Can go in with a minor fix-


libs/mediacenter/CMakeLists.txt
https://git.reviewboard.kde.org/r/117923/#comment39738

NEPOMUK_CORE_LIBRARY is not required


- Shantanu Tushar


On May 1, 2014, 7:32 a.m., Sinny Kumari wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117923/
 ---
 
 (Updated May 1, 2014, 7:32 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Currently test is getting compiled along with libs. Due to this same source 
 was compiled multiple times. With this patch, Plasma-medacenter creates a new 
 plasmamediacentertest library which has default all exported symbols from 
 libs. Now, test can be linked with libplasmamediacentertest.
 
 
 Diffs
 -
 
   libs/CMakeLists.txt a0a0f72 
   libs/mediacenter/CMakeLists.txt 2dbec83 
   libs/mediacenter/test/fakemediavalidator.h 201973c 
   libs/mediacenter/test/fakemediavalidator.cpp 9b2feca 
   libs/mediacenter/test/lastfmimagefetchertest.h 6422824 
   libs/mediacenter/test/lastfmimagefetchertest.cpp b0fcf6f 
   libs/mediacenter/test/mediacentertest.h 6997ae9 
   libs/mediacenter/test/mediacentertest.cpp 96a70a9 
   libs/mediacenter/test/medialibrarytest.h 718457d 
   libs/mediacenter/test/medialibrarytest.cpp 08af239 
   libs/mediacenter/test/mediatest.h da16e59 
   libs/mediacenter/test/mediatest.cpp 832fedb 
   libs/mediacenter/test/pmcmediatest.h 2322176 
   libs/mediacenter/test/pmcmediatest.cpp 0da8dd3 
   libs/mediacenter/test/singletonfactorytest.h 79e4197 
   libs/mediacenter/test/singletonfactorytest.cpp 308487d 
   libs/mediacenter/test/testhelpers.h 9825e85 
   libs/test/CMakeLists.txt PRE-CREATION 
   libs/test/fakemediavalidator.h PRE-CREATION 
   libs/test/fakemediavalidator.cpp PRE-CREATION 
   libs/test/lastfmimagefetchertest.h PRE-CREATION 
   libs/test/lastfmimagefetchertest.cpp PRE-CREATION 
   libs/test/mediacentertest.h PRE-CREATION 
   libs/test/mediacentertest.cpp PRE-CREATION 
   libs/test/medialibrarytest.h PRE-CREATION 
   libs/test/medialibrarytest.cpp PRE-CREATION 
   libs/test/mediatest.h PRE-CREATION 
   libs/test/mediatest.cpp PRE-CREATION 
   libs/test/pmcmediatest.h PRE-CREATION 
   libs/test/pmcmediatest.cpp PRE-CREATION 
   libs/test/singletonfactorytest.h PRE-CREATION 
   libs/test/singletonfactorytest.cpp PRE-CREATION 
   libs/test/testhelpers.h PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/117923/diff/
 
 
 Testing
 ---
 
 Application runs as expected and all unit test passed
 
 
 Thanks,
 
 Sinny Kumari
 


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


Re: Review Request 117801: Make PMC MPRIS compatible

2014-05-01 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117801/#review57048
---

Ship it!


looks good now. Please make sure to run mpristester once more before pushing to 
master.

- Shantanu Tushar


On April 27, 2014, 7:12 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117801/
 ---
 
 (Updated April 27, 2014, 7:12 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Add Dbus adaptors through which any other application can programmatically 
 control PMC using MPRIS specifications.
 http://specifications.freedesktop.org/mpris-spec/latest/index.html
 
 
 Diffs
 -
 
   libs/mediacenter/CMakeLists.txt 37769f6 
   libs/mediacenter/mpris2/mediaplayer2.h PRE-CREATION 
   libs/mediacenter/mpris2/mediaplayer2.cpp PRE-CREATION 
   libs/mediacenter/mpris2/mediaplayer2player.h PRE-CREATION 
   libs/mediacenter/mpris2/mediaplayer2player.cpp PRE-CREATION 
   libs/mediacenter/mpris2/mpris2.h PRE-CREATION 
   libs/mediacenter/mpris2/mpris2.cpp PRE-CREATION 
   mediaelements/mediaplayer/MediaPlayer.qml 39ed617 
   shells/newshell/mainwindow.h 9cbf77c 
   shells/newshell/mainwindow.cpp c1be61a 
   shells/newshell/package/contents/ui/mediacenter.qml 50f3576 
 
 Diff: https://git.reviewboard.kde.org/r/117801/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer and mpristester : 
 https://github.com/randomguy3/mpristester
 
 
 Thanks,
 
 Ashish Madeti
 


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


Review Request 117931: Restrict sourceSize to screen size

2014-05-01 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117931/
---

Review request for Plasma.


Bugs: 334176
http://bugs.kde.org/show_bug.cgi?id=334176


Repository: plasma-mediacenter


Description
---

This helps by limiting the image source width so that it doesnt load full 
versions of images with resolutions more than the screen size.


Diffs
-

  mediaelements/imageviewer/ImageViewer.qml b182bf2 

Diff: https://git.reviewboard.kde.org/r/117931/diff/


Testing
---

works with both portrait and landscape


Thanks,

Shantanu Tushar

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


Re: Review Request 117867: Do not cache media lists to a database

2014-04-30 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117867/
---

(Updated April 30, 2014, 5:27 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

We started caching results from media sources due to our primary data source 
(Nepomuk) was quite slow and the experience was quite bad both for fetching 
media and searching through it. Going forward, Baloo is our primary data 
source, and was found to be quite performant. Given this, the additional 
complexity of maintaining a cache in DB is too much for the 1-2 second increase 
in Media Library load times.

Note that we are still able to pull in data from multiple data sources, we just 
won't store it on disk.


Diffs
-

  README be3e060 
  cmake/FindQxOrm.cmake 3c1fae1 
  libs/CMakeLists.txt 32d45c2 
  libs/mediacenter/CMakeLists.txt 809d2c0 
  libs/mediacenter/album.h 64b1205 
  libs/mediacenter/album.cpp 391a420 
  libs/mediacenter/artist.h 064aa02 
  libs/mediacenter/artist.cpp 6b32269 
  libs/mediacenter/media.h 830ad57 
  libs/mediacenter/media.cpp a56a693 
  libs/mediacenter/medialibrary.h 8f9b5d3 
  libs/mediacenter/medialibrary.cpp 8f49c05 
  libs/mediacenter/pmcalbum.h 40c42fe 
  libs/mediacenter/pmcartist.h f15a2e2 
  libs/mediacenter/pmcmedia.cpp d94d6c7 
  libs/mediacenter/precompiled.h 73490b8 
  libs/mediacenter/qxorm_export.h 33fbf11 
  libs/mediacenter/test/medialibrarytest.h 2f8b476 
  libs/mediacenter/test/medialibrarytest.cpp bb20a61 
  libs/mediacenter/test/mediatest.h 965bc7e 
  libs/mediacenter/test/mediatest.cpp 4f44334 

Diff: https://git.reviewboard.kde.org/r/117867/diff/


Testing
---

Unit tests pass, functionality works as expected.


Thanks,

Shantanu Tushar

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


Review Request 117915: Fetch photo-taken Date/Time for Images and file created Date/Time for other files

2014-04-30 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117915/
---

Review request for Plasma and Vishesh Handa.


Repository: plasma-mediacenter


Description
---

Just like the Nepomuk media source, the Baloo media source needs to provide the 
date/time information for media. This is used to sort the media to show the 
more recent media first.
For images, the date/time when the photo was actually taken is used, and the 
file creation date/time is used for other media.


Diffs
-

  plugins/baloosearch/CMakeLists.txt 1ff81fb 
  plugins/baloosearch/audiosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/audiosearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/baloosearchmediasource.h e315de4 
  plugins/baloosearch/baloosearchmediasource.cpp 7ebfa61 
  plugins/baloosearch/imagesearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/imagesearchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/searchresulthandler.h PRE-CREATION 
  plugins/baloosearch/searchresulthandler.cpp PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.h PRE-CREATION 
  plugins/baloosearch/videosearchresulthandler.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/117915/diff/


Testing
---

Tested with all three types of media, works fine. Unit tests for the new code 
to follow.


Thanks,

Shantanu Tushar

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


Re: Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter

2014-04-29 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117800/
---

(Updated April 29, 2014, 9:39 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-mediacenter


Description
---

Doing so gives us two benefits-
1. We no longer have to export classes just to test them
2. We can use ORM mapped classes such as Artist, Album without extra magic for 
tests

(the diff shows deleted/new files because of the move, reviewing CMakeLists.txt 
changes should be sufficient)


Diffs
-

  libs/CMakeLists.txt 59334e9 
  libs/mediacenter/CMakeLists.txt 37769f6 
  libs/mediacenter/lastfmimagefetcher.h 4541c66 
  libs/mediacenter/media.h 2d6cc73 
  libs/mediacenter/pmcimagecache.h d6d332e 
  libs/mediacenter/subtitleprovider.h 6f0d0f6 
  libs/test/CMakeLists.txt 52ec617 
  libs/test/fakemediavalidator.h  
  libs/test/fakemediavalidator.cpp c2fdc17 
  libs/test/lastfmimagefetchertest.h  
  libs/test/lastfmimagefetchertest.cpp 9d06079 
  libs/test/mediacentertest.h  
  libs/test/mediacentertest.cpp bb4c4d1 
  libs/test/medialibrarytest.h  
  libs/test/medialibrarytest.cpp 746d5b2 
  libs/test/mediatest.h  
  libs/test/mediatest.cpp 86ec0d0 
  libs/test/pmcmediatest.h  
  libs/test/pmcmediatest.cpp 4bdcae6 
  libs/test/singletonfactorytest.h  
  libs/test/singletonfactorytest.cpp b9fc987 
  libs/test/testhelpers.h  

Diff: https://git.reviewboard.kde.org/r/117800/diff/


Testing
---

compiles, all unit tests pass


Thanks,

Shantanu Tushar

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


Review Request 117867: Do not cache media lists to a database

2014-04-29 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117867/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

We started caching results from media sources due to our primary data source 
(Nepomuk) was quite slow and the experience was quite bad both for fetching 
media and searching through it. Going forward, Baloo is our primary data 
source, and was found to be quite performant. Given this, the additional 
complexity of maintaining a cache in DB is too much for the 1-2 second increase 
in Media Library load times.

Note that we are still able to pull in data from multiple data sources, we just 
won't store it on disk.


Diffs
-

  README be3e060 
  cmake/FindQxOrm.cmake 3c1fae1 
  libs/CMakeLists.txt 32d45c2 
  libs/mediacenter/CMakeLists.txt 809d2c0 
  libs/mediacenter/album.h 64b1205 
  libs/mediacenter/album.cpp 391a420 
  libs/mediacenter/artist.h 064aa02 
  libs/mediacenter/artist.cpp 6b32269 
  libs/mediacenter/media.h 830ad57 
  libs/mediacenter/media.cpp a56a693 
  libs/mediacenter/medialibrary.h 8f9b5d3 
  libs/mediacenter/medialibrary.cpp 8f49c05 
  libs/mediacenter/pmcalbum.h 40c42fe 
  libs/mediacenter/pmcartist.h f15a2e2 
  libs/mediacenter/pmcmedia.cpp d94d6c7 
  libs/mediacenter/precompiled.h 73490b8 
  libs/mediacenter/qxorm_export.h 33fbf11 
  libs/mediacenter/test/medialibrarytest.h 2f8b476 
  libs/mediacenter/test/medialibrarytest.cpp bb20a61 
  libs/mediacenter/test/mediatest.h 965bc7e 
  libs/mediacenter/test/mediatest.cpp 4f44334 

Diff: https://git.reviewboard.kde.org/r/117867/diff/


Testing
---

Unit tests pass, functionality works as expected.


Thanks,

Shantanu Tushar

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


Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter

2014-04-27 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117800/
---

Review request for Plasma.


Repository: plasma-mediacenter


Description
---

Doing so gives us two benefits-
1. We no longer have to export classes just to test them
2. We can use ORM mapped classes such as Artist, Album without extra magic for 
tests

(the diff shows deleted/new files because of the move, reviewing CMakeLists.txt 
changes should be sufficient)


Diffs
-

  libs/CMakeLists.txt 59334e9 
  libs/mediacenter/CMakeLists.txt 37769f6 
  libs/mediacenter/test/CMakeLists.txt PRE-CREATION 
  libs/mediacenter/test/fakemediavalidator.h PRE-CREATION 
  libs/mediacenter/test/fakemediavalidator.cpp PRE-CREATION 
  libs/mediacenter/test/lastfmimagefetchertest.h PRE-CREATION 
  libs/mediacenter/test/lastfmimagefetchertest.cpp PRE-CREATION 
  libs/mediacenter/test/mediacentertest.h PRE-CREATION 
  libs/mediacenter/test/mediacentertest.cpp PRE-CREATION 
  libs/mediacenter/test/medialibrarytest.h PRE-CREATION 
  libs/mediacenter/test/medialibrarytest.cpp PRE-CREATION 
  libs/mediacenter/test/mediatest.h PRE-CREATION 
  libs/mediacenter/test/mediatest.cpp PRE-CREATION 
  libs/mediacenter/test/pmcmediatest.h PRE-CREATION 
  libs/mediacenter/test/pmcmediatest.cpp PRE-CREATION 
  libs/mediacenter/test/singletonfactorytest.h PRE-CREATION 
  libs/mediacenter/test/singletonfactorytest.cpp PRE-CREATION 
  libs/mediacenter/test/testhelpers.h PRE-CREATION 
  libs/test/CMakeLists.txt 52ec617 
  libs/test/fakemediavalidator.h 201973c 
  libs/test/fakemediavalidator.cpp c2fdc17 
  libs/test/lastfmimagefetchertest.h 6422824 
  libs/test/lastfmimagefetchertest.cpp 9d06079 
  libs/test/mediacentertest.h 6997ae9 
  libs/test/mediacentertest.cpp bb4c4d1 
  libs/test/medialibrarytest.h 2f8b476 
  libs/test/medialibrarytest.cpp 746d5b2 
  libs/test/mediatest.h 965bc7e 
  libs/test/mediatest.cpp 86ec0d0 
  libs/test/pmcmediatest.h 2322176 
  libs/test/pmcmediatest.cpp 4bdcae6 
  libs/test/singletonfactorytest.h 79e4197 
  libs/test/singletonfactorytest.cpp b9fc987 
  libs/test/testhelpers.h 9825e85 

Diff: https://git.reviewboard.kde.org/r/117800/diff/


Testing
---

compiles, all unit tests pass


Thanks,

Shantanu Tushar

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


Re: Review Request 117800: Move tests inside libs/mediacenter and make them compile with the library sources instead of linking to libplasmamediacenter

2014-04-27 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117800/
---

(Updated April 27, 2014, 4:08 p.m.)


Review request for Plasma.


Changes
---

Removed leftover CMakeLists.txt and removed unnecessary moc includes.


Repository: plasma-mediacenter


Description
---

Doing so gives us two benefits-
1. We no longer have to export classes just to test them
2. We can use ORM mapped classes such as Artist, Album without extra magic for 
tests

(the diff shows deleted/new files because of the move, reviewing CMakeLists.txt 
changes should be sufficient)


Diffs (updated)
-

  libs/CMakeLists.txt 59334e9 
  libs/mediacenter/CMakeLists.txt 37769f6 
  libs/mediacenter/lastfmimagefetcher.h 4541c66 
  libs/mediacenter/media.h 2d6cc73 
  libs/mediacenter/pmcimagecache.h d6d332e 
  libs/mediacenter/subtitleprovider.h 6f0d0f6 
  libs/test/CMakeLists.txt 52ec617 
  libs/test/fakemediavalidator.h  
  libs/test/fakemediavalidator.cpp c2fdc17 
  libs/test/lastfmimagefetchertest.h  
  libs/test/lastfmimagefetchertest.cpp 9d06079 
  libs/test/mediacentertest.h  
  libs/test/mediacentertest.cpp bb4c4d1 
  libs/test/medialibrarytest.h  
  libs/test/medialibrarytest.cpp 746d5b2 
  libs/test/mediatest.h  
  libs/test/mediatest.cpp 86ec0d0 
  libs/test/pmcmediatest.h  
  libs/test/pmcmediatest.cpp 4bdcae6 
  libs/test/singletonfactorytest.h  
  libs/test/singletonfactorytest.cpp b9fc987 
  libs/test/testhelpers.h  

Diff: https://git.reviewboard.kde.org/r/117800/diff/


Testing
---

compiles, all unit tests pass


Thanks,

Shantanu Tushar

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


Re: Review Request 117801: Make PMC MPRIS compatible

2014-04-27 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117801/#review56684
---



libs/mediacenter/CMakeLists.txt
https://git.reviewboard.kde.org/r/117801/#comment39531

maybe keep mpris2 inside mediacenter/ ?



libs/mpris2/mediaplayer2.cpp
https://git.reviewboard.kde.org/r/117801/#comment39532

Instead of keeping a copy of the mainwindow, use a signal emitted from 
MediaPlayer2 and handle this in a MainWindow slot (you'll need to relay it 
through Mpris2).
Its always a good idea to keep dependencies at the minimum.



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/117801/#comment39535

return mediaPlayerPresent() works fine for me



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/117801/#comment39533

Move this into a common function so that it can be reused at other places.



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/117801/#comment39534

if (media) {

If you use KDevelop,  goto Configure KDevelop  Source formatter and choose 
KDElibs. Then you can use Format files on your code to auto format it.
Otherwise you can use astyle.


- Shantanu Tushar


On April 27, 2014, 2:33 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117801/
 ---
 
 (Updated April 27, 2014, 2:33 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Add Dbus adaptors through which any other application can programmatically 
 control PMC using MPRIS specifications.
 http://specifications.freedesktop.org/mpris-spec/latest/index.html
 
 
 Diffs
 -
 
   libs/mediacenter/CMakeLists.txt 37769f6 
   libs/mpris2/mediaplayer2.h PRE-CREATION 
   libs/mpris2/mediaplayer2.cpp PRE-CREATION 
   libs/mpris2/mediaplayer2player.h PRE-CREATION 
   libs/mpris2/mediaplayer2player.cpp PRE-CREATION 
   libs/mpris2/mpris2.h PRE-CREATION 
   libs/mpris2/mpris2.cpp PRE-CREATION 
   mediaelements/mediaplayer/MediaPlayer.qml 39ed617 
   shells/newshell/mainwindow.cpp c1be61a 
   shells/newshell/package/contents/ui/mediacenter.qml 50f3576 
 
 Diff: https://git.reviewboard.kde.org/r/117801/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer and mpristester : 
 https://github.com/randomguy3/mpristester
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 117732: Move settings class to lib from shell

2014-04-24 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117732/#review56364
---

Ship it!


Ship It!

- Shantanu Tushar


On April 24, 2014, 6:10 a.m., Bhushan Shah wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117732/
 ---
 
 (Updated April 24, 2014, 6:10 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 As discussed on IRC.
 
 
 Diffs
 -
 
   libs/mediacenter/CMakeLists.txt f5aa827 
   libs/mediacenter/settings.h PRE-CREATION 
   libs/mediacenter/settings.cpp PRE-CREATION 
   shells/newshell/CMakeLists.txt 9d9ea29 
   shells/newshell/mainwindow.cpp 9735a0a 
   shells/newshell/settings.h 1fdcb8d 
   shells/newshell/settings.cpp ea7ea7d 
 
 Diff: https://git.reviewboard.kde.org/r/117732/diff/
 
 
 Testing
 ---
 
 Compiles
 Links
 Test passes
 Works
 
 
 Thanks,
 
 Bhushan Shah
 


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


Re: Review Request 117691: Unload libplasma after using it in Plasma Media Center

2014-04-24 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117691/#review56459
---

Ship it!


Ship It!

- Shantanu Tushar


On April 24, 2014, 5:47 p.m., Sinny Kumari wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117691/
 ---
 
 (Updated April 24, 2014, 5:47 p.m.)
 
 
 Review request for Plasma and Albert Astals Cid.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 In Kubuntu 14.04 release, we found that Plasma Media Center was unable to 
 play video/audio files. Reason behind it was PMC was linking with libplasma 
 and QtMultimediaKit. libplasma links with libgstreamer1.0 while 
 QtMultimediaKit links with libgstreamer0.10. 
 
 This patch loads libplasma and unloads it after we are done as suggested on 
 http://lists.kde.org/?l=kde-develm=139811564632441w=2 .
 
 
 Diffs
 -
 
   libs/CMakeLists.txt 7e0a9aa 
   libs/plasmaadapter/CMakeLists.txt PRE-CREATION 
   libs/plasmaadapter/plasmaadapter.cpp PRE-CREATION 
   shells/newshell/CMakeLists.txt bc40044 
   shells/newshell/mainwindow.h eb2b276 
   shells/newshell/mainwindow.cpp cfbe097 
 
 Diff: https://git.reviewboard.kde.org/r/117691/diff/
 
 
 Testing
 ---
 
 Able to play audio/video on Kubuntu 14.04 and Fedora 19. 
 
 
 Thanks,
 
 Sinny Kumari
 


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


Loading and unloading a shared library after use

2014-04-21 Thread Shantanu Tushar Jha
Hi fellow devs,

In the latest Kubuntu release, 14.04, Plasma Media Center's primary feature
- playback of audio and video has stopped working. This is because we link
to libplasma which in turn links to libgstreamer-1.0 while QtMultimediaKit
links to libgstreamer-0.10. This upsets Glib because it doesn't want the
same binary load both gstreamer-1.0 and gstreamer-0.10[1]

Now, we use (and link to) Plasma for two things-

   - Setting the Oxygen theme -
   Plasma::Theme::defaultTheme()-setThemeName(oxygen)
   - Load PMC's main QML file and images which are part of a Plasma::Package

Removing these two from the code, and removing the link to libplasma, we
can get PMC to play stuff fine. However, this means we 1. use the Air theme
which doesn't really look very nice[2] and 2. we need to figure out another
way to locate the installed QML files. The latter is workable, but we
really want to be able to use Oxygen Plasma theme.

We do both these things when the app starts, so I am wondering if there is
a way we could dynamically load libplasma, make these two method calls, and
somehow unload libplasma. I have two questions -


   - Does that ^ even make sense? Will it solve the problem?
   - If yes, how exactly do you do it?

Help appreciated, so that our users can play their audio and video again ;)

Cheers,
[1]
http://cgit.freedesktop.org/gstreamer/gstreamer/tree/docs/random/porting-to-1.0.txtsection
Troubleshooting
[2] http://i.imgur.com/0k1TTwW.png

-- 
Shantanu Tushar(UTC +0530)
http://www.shantanutushar.com
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 117368: Resolve first-time-run crash of PMC due to not existing data directory.

2014-04-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117368/#review54978
---

Ship it!


Looks good to me, I have one suggestion below. Make sure that all tests pass 
before pushing.


libs/mediacenter/mediacenter.cpp
https://git.reviewboard.kde.org/r/117368/#comment38361

const QString pmc_path


- Shantanu Tushar


On April 4, 2014, 11:42 a.m., Nikolaos Chatzidakis wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117368/
 ---
 
 (Updated April 4, 2014, 11:42 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This patch resolves the crash of PMC if it's running for the first time and 
 its data directory does not exist in ~/.kde4/share/apps.
 
 
 Diffs
 -
 
   libs/mediacenter/mediacenter.cpp 35240d3 
 
 Diff: https://git.reviewboard.kde.org/r/117368/diff/
 
 
 Testing
 ---
 
 Tested the fix, it creates the directory if it doesn't exist (first-time-run 
 assumption).
 
 
 Thanks,
 
 Nikolaos Chatzidakis
 


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


Re: Review Request 117365: Add Genre to metadata PMC keeps

2014-04-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117365/#review54982
---


Works fine, I can see the values in the table. However, the actual set of the 
value should happen inside Media instead of MediaLibrary as the comments below.


libs/mediacenter/media.h
https://git.reviewboard.kde.org/r/117365/#comment38362

doesnt need to be const QString for the return, just QString will do.
The const QString you see at other places is quite some bad luck and needs 
to be fixed to QString as well, separately.
For more 
http://techbase.kde.org/Policies/Library_Code_Policy#Const_References



libs/mediacenter/medialibrary.cpp
https://git.reviewboard.kde.org/r/117365/#comment38363

Instead of handling this here, you should let media-setValueForRole take 
care of updating the value. Just implement MediaCenter::GenreRole in 
Media::setValueForRole.

Also, if you set wasUpdated to true always, the point of wasUpdated is 
lost. It is supposed to be set to true only when the old value and new value 
are different (which is when the setValueForRole method is supposed to return 
true).

At the end, this else if should be removed (notice how there is no else if 
for createdAt (extractAndSaveDurationInfo should not have been there as well, 
it was missed and should be removed separately).



libs/mediacenter/medialibrary.cpp
https://git.reviewboard.kde.org/r/117365/#comment38364

same as above



plugins/kdedesktopsearch/kdemetadatamediasource.cpp
https://git.reviewboard.kde.org/r/117365/#comment38365

Just for consistency, move this to the line after CreatedAtRole


- Shantanu Tushar


On April 4, 2014, 10:51 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117365/
 ---
 
 (Updated April 4, 2014, 10:51 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Added genre to metadata pmc keeps so that it too can be retrieved from the 
 media library when needed.
 
 
 Diffs
 -
 
   libs/mediacenter/media.h 2bdef4d 
   libs/mediacenter/media.cpp 32b9f19 
   libs/mediacenter/mediacenter.h a2855dd 
   libs/mediacenter/mediacenter.cpp 35240d3 
   libs/mediacenter/medialibrary.cpp 713827a 
   libs/mediacenter/pmcmedia.h 68275f2 
   libs/mediacenter/pmcmedia.cpp a266a26 
   libs/mediacenter/pmcmetadatamodel.cpp 2d5fd6b 
   plugins/kdedesktopsearch/kdemetadatamediasource.cpp e8f18eb 
 
 Diff: https://git.reviewboard.kde.org/r/117365/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 117365: Add Genre to metadata PMC keeps

2014-04-04 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117365/#review55005
---

Ship it!


Ship It!

- Shantanu Tushar


On April 4, 2014, 1:02 p.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117365/
 ---
 
 (Updated April 4, 2014, 1:02 p.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Added genre to metadata pmc keeps so that it too can be retrieved from the 
 media library when needed.
 
 
 Diffs
 -
 
   libs/mediacenter/media.h 2bdef4d 
   libs/mediacenter/media.cpp 32b9f19 
   libs/mediacenter/mediacenter.h a2855dd 
   libs/mediacenter/mediacenter.cpp 35240d3 
   libs/mediacenter/pmcmedia.h 68275f2 
   libs/mediacenter/pmcmedia.cpp a266a26 
   libs/mediacenter/pmcmetadatamodel.cpp 2d5fd6b 
   plugins/kdedesktopsearch/kdemetadatamediasource.cpp e8f18eb 
 
 Diff: https://git.reviewboard.kde.org/r/117365/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ashish Madeti
 


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


Re: Review Request 116874: Patch for the next and previous button functionality in an inactive playlist.

2014-03-27 Thread Shantanu Tushar


 On March 27, 2014, 1:05 p.m., Sebastian Kügler wrote:
  mediaelements/playlist/Playlist.qml, line 159
  https://git.reviewboard.kde.org/r/116874/diff/3/?file=257374#file257374line159
 
  indenting is wrong now
 
 R.Harish  Navnit wrote:
 I have added a new diff file after I pulled the latest source and built 
 successfully. Please do check that 
 patch(https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch),
  I've added {} to the if-statement in that one. It however, isn't disabled 
 when the playlist isn't active.

Can you update the diff in the review instead of attaching it? Otherwise its 
not possible to leave comments on code.


- Shantanu


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116874/#review54293
---


On March 27, 2014, 11:22 a.m., R.Harish  Navnit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116874/
 ---
 
 (Updated March 27, 2014, 11:22 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar and Sinny Kumari.
 
 
 Bugs: 330990 and Bug
 http://bugs.kde.org/show_bug.cgi?id=330990
 http://bugs.kde.org/show_bug.cgi?id=Bug
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Now the next and previous buttons do not work unless a media is playing. 
 
 
 Diffs
 -
 
   mediaelements/playlist/Playlist.qml fd83c21 
 
 Diff: https://git.reviewboard.kde.org/r/116874/diff/
 
 
 Testing
 ---
 
 I've done the testing for this patch.
 
 These are the steps that I followed. 
 1. Added a new playlist and added a few songs to the playlist.
 2. Re-opened Plasma Media Center 
 3. Clicked the Next and Previous buttons.
 4. Nothing was played.
 5. Selected a song from the list and then tested the next and previous 
 buttons.
 6. The buttons worked seamlessly.  
 
 
 File Attachments
 
 
 pulled the latest source and made changes
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/03/27/7f72b75b-6d72-4a5f-9460-699bb3846624__updated_fix.patch
 
 
 Thanks,
 
 R.Harish  Navnit
 


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


Re: Review Request 117101: This patch fixes font size behaviour when pmc is resized.

2014-03-27 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117101/#review54353
---


Doesn't really look on my box http://i.imgur.com/nJMYi9E.jpg
Also, I think the right fix for these things is to use one of 
PlasmaExtras.Heading, PlasmaExtras.Title (or PlasmaExtras.Paragraph when 
applicable). Can someone else comment on whether making the font depend on 
window size is a good idea?

- Shantanu Tushar


On March 26, 2014, 9:29 p.m., Nikolaos Chatzidakis wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117101/
 ---
 
 (Updated March 26, 2014, 9:29 p.m.)
 
 
 Review request for Plasma and Shantanu Tushar.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 This is my first patch trying to hack PMC. Untill now, whenever PMC was 
 resized, the menu strings font size remained fixed. Now it changes the font 
 size according to parents dimensions.
 
 
 Diffs
 -
 
   mediaelements/mediawelcome/BackendsListDelegate.qml 4840982 
   mediaelements/mediawelcome/MediaWelcome.qml a3108d8 
 
 Diff: https://git.reviewboard.kde.org/r/117101/diff/
 
 
 Testing
 ---
 
 The size of the icons in the menus remains the same, due to the file format 
 (png). Converting these graphics to SVG may solve the graphics resize issue 
 as well.
 
 
 Thanks,
 
 Nikolaos Chatzidakis
 


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


Re: Review Request 116907: Implemented Player MPRIS spec adaptor

2014-03-26 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116907/#review54208
---


I have general feedback around the code below. It will be nice if you can poke 
someone who has written MPRIS-related code before to review this before 
finalizing.


libs/mpris2/mediaplayer2.cpp
https://git.reviewboard.kde.org/r/116907/#comment37904

This would look better-

QDBusConnection::sessionBus().registerObject(/org/mpris/MediaPlayer2, 
this, 
QDBusConnection::ExportAllSlots |
QDBusConnection::ExportAllProperties |
QDBusConnection::ExportAllSignals);



libs/mpris2/mediaplayer2player.h
https://git.reviewboard.kde.org/r/116907/#comment37906

Are these supposed to be called on this object from outside? If yes, why 
are these protected? If no, why Q_INVOKABLE?



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/116907/#comment37907

How is 64 decided?



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/116907/#comment37909

if and else need braces, even when single-line, see 
http://techbase.kde.org/Policies/Kdelibs_Coding_Style



libs/mpris2/mediaplayer2player.cpp
https://git.reviewboard.kde.org/r/116907/#comment37910

const int



shells/newshell/mainwindow.cpp
https://git.reviewboard.kde.org/r/116907/#comment37911

whitespace



shells/newshell/package/contents/ui/mediacenter.qml
https://git.reviewboard.kde.org/r/116907/#comment37912

instead of this, can't you use onPlayRequested instead? Its better to avoid 
dependencies for components (In this case Playlist requires a mprisPlayer to 
work).


- Shantanu Tushar


On March 26, 2014, 6:46 a.m., Ashish Madeti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116907/
 ---
 
 (Updated March 26, 2014, 6:46 a.m.)
 
 
 Review request for Plasma, Shantanu Tushar, Sinny Kumari, and Sujith 
 Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Implemented Player DBus adaptor of MPRIS specifications for Plasma 
 Mediacenter.
 Specification reference: 
 http://specifications.freedesktop.org/mpris-spec/latest/Player_Interface.html
 
 Some more work needs to be done in the adaptor which I plan to do soon.
 
 
 Diffs
 -
 
   libs/mpris2/mpris2.cpp a8ad3ef 
   mediaelements/mediaplayer/MediaPlayer.qml 39ed617 
   mediaelements/playlist/Playlist.qml 5dde297 
   shells/newshell/mainwindow.cpp d2d71d4 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
   libs/mpris2/mediaplayer2.h e68bc5c 
   libs/mpris2/mediaplayer2.cpp ff96618 
   libs/mpris2/mediaplayer2player.h 203d681 
   libs/mpris2/mediaplayer2player.cpp 7871efa 
 
 Diff: https://git.reviewboard.kde.org/r/116907/diff/
 
 
 Testing
 ---
 
 Tested with qdbusviewer, the properties and methods are working fine.
 
 
 Thanks,
 
 Ashish Madeti
 


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


Review Request 116898: Get rid of unnecessary Q_INVOKABLE declarations

2014-03-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116898/
---

Review request for Plasma and Sujith Haridasan.


Repository: plasma-mediacenter


Description
---

Methods supposed to be used as READ of Q_PROPERTY properties need not be 
invokable. Same goes for slots.


Diffs
-

  libs/mpris2/mediaplayer2.h e68bc5c 
  libs/mpris2/mediaplayer2player.h 203d681 
  libs/mpris2/mpris2.h 0df64f4 
  libs/mpris2/mpris2.cpp a8ad3ef 
  shells/newshell/main.cpp bab6915 
  shells/newshell/mainwindow.cpp d2d71d4 

Diff: https://git.reviewboard.kde.org/r/116898/diff/


Testing
---


Thanks,

Shantanu Tushar

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


Re: Review Request 116554: Implementation of autoplay when the PlayAll button in pressed

2014-03-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116554/#review53438
---



shells/newshell/mainwindow.cpp
https://git.reviewboard.kde.org/r/116554/#comment37583

This processCommandLineArgs call (only the call) is still needed, otherwise 
media passed as cmd line args wont play.
Please test before updating diff.


- Shantanu Tushar


On March 19, 2014, 6:30 a.m., Harshit Agarwal wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116554/
 ---
 
 (Updated March 19, 2014, 6:30 a.m.)
 
 
 Review request for Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and 
 Sujith Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Referring to Bug #331040 The functionality of the media starting to play on 
 the press of the Play All button has been implemented.
 It has been noted and implemented that the media doesn't start over when new 
 songs/videos are added to an existing playlist. 
 
 
 Diffs
 -
 
   browsingbackends/localfiles/localfilesabstractbackend.cpp faaafa7 
   shells/newshell/mainwindow.h f224060 
   shells/newshell/mainwindow.cpp f721d68 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
 
 Diff: https://git.reviewboard.kde.org/r/116554/diff/
 
 
 Testing
 ---
 
 The testing has been done through various test scenarios.
 
 
 Thanks,
 
 Harshit Agarwal
 


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


Re: Review Request 116554: Implementation of autoplay when the PlayAll button in pressed

2014-03-19 Thread Shantanu Tushar

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116554/#review53472
---

Ship it!


Ship It!

- Shantanu Tushar


On March 19, 2014, 6:40 p.m., Harshit Agarwal wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116554/
 ---
 
 (Updated March 19, 2014, 6:40 p.m.)
 
 
 Review request for Plasma, Akshay Ratan, Shantanu Tushar, Sinny Kumari, and 
 Sujith Haridasan.
 
 
 Repository: plasma-mediacenter
 
 
 Description
 ---
 
 Referring to Bug #331040 The functionality of the media starting to play on 
 the press of the Play All button has been implemented.
 It has been noted and implemented that the media doesn't start over when new 
 songs/videos are added to an existing playlist. 
 
 
 Diffs
 -
 
   browsingbackends/localfiles/localfilesabstractbackend.cpp faaafa7 
   shells/newshell/mainwindow.h f224060 
   shells/newshell/mainwindow.cpp f721d68 
   shells/newshell/package/contents/ui/mediacenter.qml bac33c2 
 
 Diff: https://git.reviewboard.kde.org/r/116554/diff/
 
 
 Testing
 ---
 
 The testing has been done through various test scenarios.
 
 
 Thanks,
 
 Harshit Agarwal
 


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


  1   2   3   4   5   >