ngettext() with more than one number

[prl]

I'm in the process of getting the media player to stop telling untruths about the number of subdirectories in a folder when you move a folder into trash. As part of that I want to rewrite:

Code: Select all

are_you_sure = _("Do you really want to move this to trash?")
    ...
    ... [MessageBox text parameter] _("'%s' contains %d file(s) and %d sub-directories.\n") % (folder_filename, files, subdirs) + are_you_sure

to use ngettext() for the singular and plural forms. In fact, I want to move things around a bit more, like:

Code: Select all

are_you_sure = _("Do you really want to move '%s' to trash?") % folder_filename
    ...
    ...  [MessageBox text parameter] are_you_sure + "\n" + _("It contains %d file(s) and %d sub-directories.\n") % (files, subdirs)

However, ngettext() only allows for a single variable. The online advice I've seen is to re-write such things as two separate ngettext() strings and as standalone "sentences", so for example, something like:

Code: Select all

[MessageBox text parameter] are_you_sure + "\n" +ngettext(_("%d files."), _("%d file."), files) + ngettext(_(" %d sub-directory."), _(" %d sub-directories."), subdirs)

That seems a bit abrupt compared to the original, though at least it handles singular/plural correctly. Any other suggestions?

[IanSav]

Hi Prl,

Perhaps prepare the two strings as variables and then use the variables in a single and cleaner MessageBox.

Regards,
Ian.

[prl]

...
Perhaps prepare the two strings as variables and then use the variables in a single and cleaner MessageBox.
...

I'm not sure how that differs in effect from what I've proposed. Surely:

Code: Select all

    [MessageBox text parameter] are_you_sure + "\n" +ngettext(_("%d files."), _("%d file."), files) + ngettext(_(" %d sub-directory."), _(" %d sub-directories."), subdirs)

does just the same thing as:

Code: Select all

    fileText = ngettext(_("%d files."), _("%d file."), files)
    dirText = ngettext(_(" %d sub-directory."), _(" %d sub-directories."), subdirs)
    [MessageBox text parameter] are_you_sure + "\n" + fileText + dirText

That's not really what I'm asking. I'm asking if there are better ways of constructing what goes into the text, not about how clean the code looks.

For example, I don't think it's necessarily a good thing to do it as:

Code: Select all

    fileText = ngettext(_("It contains %d files."), _("It contains %d file."), files)
    dirText = ngettext(_("and %d sub-directory."), _("and %d sub-directories."), subdirs)

because that may assume too much about how conjunctions are used.

[IanSav]

Hi Prl,

As you guessed I was looking at code readability not grammatical syntax for different languages.

Would a message like:

Code: Select all

    fileText = ngettext(_("%d files."), _("%d file."), files)
    dirText = ngettext(_("%d sub-directory."), _("%d sub-directories."), subdirs)
    [MessageBox text parameter] are_you_sure + "\nIt contains:\n    " + fileText + "\n    " + dirText

help? That is, turn it into a dot point list.

Regards,
Ian.

[prl]

It's just that form (of output, not of code) I was asking for suggestions to improve on.

[IanSav]

Hi Prl,

Hence the dot point form I suggested.

Regards,
Ian.

[prl]

... Since the dot point form I suggested. ...

Sorry, I missed the changes in the construction of the text line for the MessageBox

I'll give it a try and see how it looks.

[prl]

There's another anomaly in the deletion of folders in the media player.

If "Use trash in movie list" is enabled, and the current directory isn't in the Trash*, then when you press RED Delete an empty folder, it asks whether you want to move it to trash or not (it's the clumsy implementation of this that leads to the incorrect number of sub-directories being reported in the first place).

If "Use trash in movie list" is disabled, and the current directory is in the Trash*, when you press RED Delete an empty folder, it deletes the folder without asking, even though this is a more destructive operation than moving it to trash.

I think the two cases should be handled in the same way. I don't have any strong feelings either way about which, but if anyone does, I'll probably do it that way.

* And/or if the name of the folder being deleted doesn't contain/contains the string ".Trash"

[IanSav]

Hi Prl,

To be sure I follow what you are describing and what you propose to do about the issue could you please provide a list / table of items (files / folders), location, settings and desired action / result.

Regards,
Ian.

[prl]

Hi Prl,

To be sure I follow what you are describing and what you propose to do about the issue could you please provide a list / table of items (files / folders), location, settings and desired action / result.

Regards,
Ian.

Action on pressing RED Delete on an empty folder in the media viewer:

Code: Select all

"Use trash     | Viewing Trash    | Deleted folder   | Ask before
in movie list" | or folder inside | name contains    | deleting
               | Trash            | ".Trash"         |
---------------+------------------+------------------+------------ 
enabled        | false            | false            | yes
enabled        | true             | false            | no
enabled        | false            | true             | no
enabled        | true             | true             | no
disabled       | false            | false            | no
disabled       | true             | false            | no
disabled       | false            | true             | no
disabled       | true             | true             | no

I think the last column should be either all "yes" or all "no".

[IanSav]

Hi Prl,

I think the question regarding asking the user to confirm a manual delete of a deleted item should always be "Yes". Any manual deletions within .Trash should be confirmed.

As to the issue of deleting an item that is already in the trash then I would look at appending a sequence number to the base part of the file name (before the "." and extension) of the newly deleted item so that each item in the trash has a unique name and can be correctly retrieved albeit with a slightly changed name.

Regards,
Ian.

[prl]

...
I think the question regarding asking the user to confirm a manual delete of a deleted item should always be "Yes". Any manual deletions within .Trash should be confirmed.

The exception in the current code I was asking about was for moving an item to trash, not deleting it permanently. The table is only for empty folders. I think that the logic of other cases is OK.

As to the issue of deleting an item that is already in the trash then I would look at appending a sequence number to the base part of the file name (before the "." and extension) of the newly deleted item so that each item in the trash has a unique name and can be correctly retrieved albeit with a slightly changed name.
...

That would have the side-effect of fixing bugs associated with deleting directories with the same name more than once.

Deleting recordings is typically not a problem that would often benefit from that solution, because recording names are kept fairly distinct. That's probably similar for media files.

Anyway, it's way out of scope for what I'm intending to do at the moment, and should probably have some discussion leading to an enhancement request in the issue tracker.

[peteru]

I think the two cases should be handled in the same way. I don't have any strong feelings either way about which, but if anyone does, I'll probably do it that way.

Which way will result in the smallest number of changes in the code?

It's already challenging enough to handle merges with the current improvements. Further changes will probably make it more likely that future merges will lead to bugs.

I suppose similar commentary goes for the ngettext use. Is it really necessary? If so, try to keep the changes minimal and merge friendly.

[prl]

I think the two cases should be handled in the same way. I don't have any strong feelings either way about which, but if anyone does, I'll probably do it that way.

Which way will result in the smallest number of changes in the code?

It's already challenging enough to handle merges with the current improvements. Further changes will probably make it more likely that future merges will lead to bugs.

I suppose similar commentary goes for the ngettext use. Is it really necessary? If so, try to keep the changes minimal and merge friendly.

The changes are pretty small anyway. As for using ngettext, the current text is internally inconsistent anyway, using "%d file(s)" for the number of files and '%d directories" for the number of sub-directories.

The changes are a small enough effort that I'm willing to submit them knowing I they may be at least partly rejected.

[prl]

Actually, the more I look at it, the more of a mess MovieSelection.delete() seems.

There's a lot of streamlining of the logic and changing of messages that should be done, including some quite misleading messages, like displaying "'directory' contains N file(s) and M sub-directories. Do you really want to permanently remove everything from trash?" when the action is really just to delete the single entry 'directory' from the trash.

I'm just going to implement request #326: Add optional warning popup when moving recordings/files to trash in Movie Player and leave it at that for now.