> On March 5, 2013, 12:48 p.m., Matěj Laitl wrote: > > src/services/lastfm/LastFmServiceSettings.cpp, lines 255-256 > > <http://git.reviewboard.kde.org/r/109283/diff/1/?file=116983#file116983line255> > > > > I think this will duplicate the label in case it is already in the > > combo box. (but do test it instead of fixing it blindly) > > Vedant Agarwala wrote: > By default duplicates are disbaled in QComboBox. It needs to be enabled > by setDuplicatesEnabled. That is why I didn't check if the label already > exists in the QComboBox.
To quote the docs: > duplicatesEnabled : bool > This property holds whether the user can enter duplicate items into the > combobox. > > Note that it is always possible to programmatically insert duplicate items > into the combobox. And here you indeed insert them programatically. > On March 5, 2013, 12:48 p.m., Matěj Laitl wrote: > > src/services/lastfm/ScrobblerAdapter.cpp, line 283 > > <http://git.reviewboard.kde.org/r/109283/diff/1/?file=116985#file116985line283> > > > > In fact, there's no need to check whether skipLabel() is empty. Instead > > you MUST check whether selectiveScrobbling (raname pending) is true. Also > > document that this method does the check in its docstring. > > Vedant Agarwala wrote: > Right. But I think we have to check for both because someone might check > the check box but leave the combo box empty. Checking for skipLabel().isEmpty() won't hurt, but just note the code would work even without as you'd compare track labels against empty label which would be always false. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109283/#review28575 ----------------------------------------------------------- On March 4, 2013, 7:31 p.m., Vedant Agarwala wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109283/ > ----------------------------------------------------------- > > (Updated March 4, 2013, 7:31 p.m.) > > > Review request for Amarok. > > > Description > ------- > > Added a check-box to disable scobbling for tracks with a particular label. > The label can be selected from a drop-down list or entered manually. Any > track that contains this label is skipped from being submitted to last.fm for > being scrobbled. > > > Diffs > ----- > > src/core/collections/QueryMaker.h a30b94f > src/services/lastfm/LastFmConfigWidget.ui 5c5e51b > src/services/lastfm/LastFmServiceConfig.h 3f33b72 > src/services/lastfm/LastFmServiceSettings.h 41b2ead > src/services/lastfm/LastFmServiceSettings.cpp f6e1564 > src/services/lastfm/ScrobblerAdapter.h ea74196 > src/services/lastfm/ScrobblerAdapter.cpp b1a09f8 > src/statsyncing/Process.cpp c42fdc4 > src/statsyncing/ScrobblingService.h 971edd7 > > Diff: http://git.reviewboard.kde.org/r/109283/diff/ > > > Testing > ------- > > Builds and runs successfully. > > > Thanks, > > Vedant Agarwala > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
