Re: [linux-minidisc] creating new directories in file browser

[Michael Karcher <Michael.Karcher@fu-berlin.de>]

Am Mittwoch, den 09.06.2010, 17:49 +0200 schrieb Thomas Arp:
> -------- Original-Nachricht --------
> > Datum: Wed, 9 Jun 2010 14:48: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] creating new directories in file browser
> 
> > Hi Thomas,
> > 
> > On Wed, Jun 09, 2010 at 07:42:10AM +0200, Thomas Arp wrote:
> > > Hello,
> > > i added new features to create new directories and remove files or 
> > > directories.
> > > This is done by implementing a context menu in the local browser.
> > > "new folder" will open a file dialog where you can add new directories.
> > > I decided to use QFileDialog::DontUseNativeDialog flag because on
> > > windows the native dialog don´t support to rename the new folder 
> > > during creation.
> > 
> > I've tested and reviewed your patch now. Here are my remarks:
> > 
> > - the patch misses equivalent buttons and entries to create new/remove
> >   folder in menu- and toolbar; having these functions in the context
> >   menus only is unintuitive. I'm not even sure whether context menus
> >   make sense in this context
> 
> O.K., i will change this by moving the actions to qhimdmainwindow.ui.
> What about adding buttons  at the top of the local browser window at 
> the right side instead of adding them to the toolbar?
> Most of the actions in the toolbar are for interaction with the himd 
> device but these actions only take effect on the local browser /.
> the local file system.
> 
> > - the context menus have written the command names in all lower case:
> >   it should read "Remove", not "remove" for example
> 
> O.K.
> 
> > - the mechanism to create a new folder by abusing a browse directory
> >   (QFileDialog::getExistingDirectory) dialog is a very bad design;
> >   instead it should be possible to create the new folder directly in
> >   the tree view or at least a minimal requester should pop up to ask
> >   for the new folders name (creating a new folder shouldn't be rocket
> >   science ;))
> 
> We can use localmodel.mkdir() using the current index as parent and 
> ask for the name.
> This is the way i first implemented this feature, but you cannot change
> the location of the new folder when you have clicked the action.
> First the user has to select it (currentIndex() is the default).  
> 
> > - the string for this requester is chunked into several pieces:
> > 
> >     ret = QMessageBox::question(this, tr("Removing file ordirectory"),    
> >                                                                           
> >                    
> >                                 tr("Are you sure you want to remove") +
> > file + tr(" ?\n\n") +                                                        
> >                    
> >                                 tr("Note: The file or directory will be
> > removed finally ") +                                                         
> >                     
> >                                 tr("and cannot be recovered"),            
> >                                                                           
> >                     
> > 
> > - chunking the message text of this dialog into several pieces will
> >   make translation errorprone and cumbersome; please keep strings to
> >   be tranlated contigous whenever possible
> > 
> I made this to avoid too long lines in the source code.
> We can use a "const QString warning = tr("Note: ...") ".

The important point is not to split what you pass into "tr", because
each argument to "tr" (for translate) is interpreted as a different
string to translate. So "and cannot be removed" is going to be
independently translated from the previous half-sentence "Note: The file
or directory will be removed finally ".

If it is just about avoiding long lines, please consider making use the
property of C/C++ that string literals written next to each other are
implicitly joined, like this

question(this, tr("Are you sure you want to remove %1?\n\n"
                  "Note: The file or directory will be removed permanently"
                  "and cannot be recovered.").arg(file))

Please also insert the file name by using a place holder so the
placeholder can be moved around by translators.

Regards,
  Michael Karcher