-------- Original-Nachricht -------- > Datum: Sat, 5 Jun 2010 11:11:02 +0200 > Von: Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > An: Thomas Arp <manner.moe@gmx.de> > CC: linux-minidisc@lists.fu-berlin.de > Betreff: Re: [linux-minidisc] saving window geometry > On Sat, Jun 05, 2010 at 10:44:09AM +0200, Thomas Arp wrote: > > Hi, > > i added a small feature to save the window geometry and > > column width of the himd browser when closed and restores > > them at next startup. > > Looks good to me. > > Could you re-format the patch? There are some places where you "patch" > whitespaces like lines 19 and 20 in the patch file: > > @@ -154,7 +154,7 @@ QString QHiMDMainWindow::dumppcm(const QHiMDTrack > & track, QString file) > > while(himd_nonmp3stream_read_block(&str, &data, &len, NULL, > &status) >= 0) > { > - > + > O.K., i think we have to check the code for correct line endings. Maybe it is a bug in msysgit. These "patches" are present just by creating a new branch. I checked the code with Textpad showing whitespaces and line endings. There are some blancs in the master branch at these lines which are not present on other branches. > Also: > > void QHiMDMainWindow::save_window_settings() > { > int i = 0; > > settings.setValue("geometry", QMainWindow::saveGeometry()); > settings.setValue("windowState", QMainWindow::saveState()); > for(;i == 6; i++) > > Shouldn't we rather read the number of columns in the list view rather > than assuming it's 6? I think using a constant here is more prone to > errors when changing something. > Good point, i will change this. > Otherwise I think the patch is fine and I would apply it after you > fixed the suggested changes or come up with some pretty good arguments > ;). > > Adrian Thomas