Improve wildcard handling for `imgmount` file arguments #2270

kcgen · 2023-02-09T01:37:28Z

Previously, alpha-numeric sorting was used when mounting disk images with wildcards.

This works fine when disk image numbers are zero-padded up to the largest, for example in an 11 disk set: disk01.img, disk02.img, ..., disk11.img.

However, sometimes disks aren't zero-padded resulting in unwanted ordering:

This PR adds natural sorting:

And nicer logging of the result.

Singles:
Pairs:
Three or more:

It also adds filename to the console log messages, which can help when reporting issues or when parsing with a script:

2023-02-08 | FAT: Mounted disk1.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk2.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk3.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk4.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk5.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk6.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk7.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk8.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk9.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk10.img as FAT12 volume with 2847 clusters
2023-02-08 | FAT: Mounted disk11.img as FAT12 volume with 2847 clusters

Thanks to @Eulisker from the eXoDOS project for catching this bug 👍 !

src/dos/program_imgmount.cpp

src/misc/string_utils.cpp

include/string_utils.h

johnnovak · 2023-02-09T06:54:25Z

So this log message reads fine:

FAT: Mounted disk4.img as FAT12 volume with 2847 clusters

But these I find weird:

Drive A mounted? Should be the other way around, no?

disk1.img, disk2.img, and disk3.img mounted as drive A

kcgen · 2023-02-09T18:55:06Z

Drive A mounted? Should be the other way around, no?
disk1.img, disk2.img, and disk3.img mounted as drive A

I agree.. maybe they wanted the letter to be shown first (it stands out a bit more). but I agree, yours is more natural and preferred.

I swapped the drive letter and string:

MSG_Add("PROGRAM_MOUNT_STATUS_2","Drive %c is mounted as %s\n");

to 

MSG_Add("PROGRAM_MOUNT_STATUS_2","Images %s mounted as drive %c\n");

And it segfaults (because now the arguments need to be swapped) in these usage points:

src/dos/program_imgmount.cpp
src/dos/program_mount.cpp        
src/dos/program_mount_common.cpp

That's not a big deal 🙂

The bigger hit is that all the foreign strings (for PROGRAM_MOUNT_STATUS_2) would need to be flipped as well (otherwise those users will segfault):

contrib/resources/translations/utf-8/de.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/en.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/es.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/fr.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/it.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/nl.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/pl.txt::PROGRAM_MOUNT_STATUS_2
contrib/resources/translations/utf-8/ru.txt::PROGRAM_MOUNT_STATUS_2

I'd either have to hack those strings (and do a bad translation job) or ask the translators to fix them. Unfortunately this is feeling too stuck right now for this PR - might have to forgo this change 😭

rderooy · 2023-02-09T19:12:22Z

The bigger hit is that all the foreign strings (for PROGRAM_MOUNT_STATUS_2) would need to be flipped as well (otherwise those users will segfault):

contrib/resources/translations/utf-8/de.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/en.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/es.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/fr.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/it.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/nl.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/pl.txt::PROGRAM_MOUNT_STATUS_2

contrib/resources/translations/utf-8/ru.txt::PROGRAM_MOUNT_STATUS_2

I'd either have to hack those strings (and do a bad translation job) or ask the translators to fix them. Unfortunately this is feeling too stuck right now for this PR - might have to forgo this change sob

Dutch
Old:

:PROGRAM_MOUNT_STATUS_2
Schijf %c is gekoppeld als %s

New:

:PROGRAM_MOUNT_STATUS_2
Image %s gekoppeld als schijf %c

German
Old:

:PROGRAM_MOUNT_STATUS_2
Laufwerk %c ist eingebunden als %s

New:

:PROGRAM_MOUNT_STATUS_2
Image %s ist eingebunden als Laufwerk %c

kcgen · 2023-02-09T19:16:41Z

@rderooy : one man translation team! Awesome.

Will go ahead with this :-)

rderooy · 2023-02-09T20:34:26Z

Thanks ;-)

I'm less fluent in French and Italian, but I think this should be correct...

Original French

Lecteur %c monté en tant que %s

Updated:

Image %s monté en tant que lecteur %c

Original Italian

Unità %c montata come %s

Updated:

Immagine %s montata come unità %c

kcgen · 2023-02-09T20:42:19Z

Less fluent is far better than zero fluency for myself! Thanks for those @rderooy.

Russian, Spanish, And Polish left to go! 🔥

kcgen · 2023-02-09T20:44:37Z

@rderooy : we need conjunction words for these languages, similar to how "and" is used:

Apple, banana, and pear.

I will add just that word as its own translated string, so we can use it in other translations too.

rderooy · 2023-02-09T21:05:08Z

Spanish, possibly. Close enough to Italian. Polish and Russian, no way ;-)

rderooy · 2023-02-09T21:09:05Z

Conjunction words:

Dutch: en
German: und
French: et
Italian: e
Spanish: y

rderooy · 2023-02-09T21:15:49Z

I think the Spanish translation would be

Imagen %s montada como unidad %c

FeralChild64 · 2023-02-09T21:23:59Z

Conjunction words:

Polish: i

Translation - old:

:PROGRAM_MOUNT_STATUS_2
Dysk %c jest zamontowany jako '%s'.

Translation - new:

:PROGRAM_MOUNT_STATUS_2
Obraz %s zamontowany jako dysk %c.

I'll need to review the Polish translation once again, there are probably more cases where the single quotation marks or the verbs should be removed (like in this case).

@rderooy French translation looks OK to me - but I'm also not fluent with this language.

kcgen · 2023-02-09T22:15:29Z

@GranMinigun

help needed to translate "and" conjunction to Russian:

So far, I've got и (versus а, which contrasts)?

Also tidies up superfluous path specifications to reduce visual clutter in screen and log output.

Includes: - Uses more readable comma-separated format - Adds the filenames to the console log messages

kcgen · 2023-02-10T01:48:52Z

@johnnovak , @rderooy , @FeralChild64 , @GranMinigun , @NicknineTheEagle , @Kappa971 , @Burrito78

Here are the updated mount status logs, do they look OK?

Regarding "drive C versus C drive", the latter is more correct so I've used that for English and Dutch, as my searches found those were the most prevalent and acceptable.

My searches in the other languages show the "drive C" prefix-approach is more common. Please shout if I got this wrong or if there are more tweaks.

johnnovak · 2023-02-10T03:07:40Z

Looks good to me 👍🏻 Minor comment: there's a period at the end of the Polish (?) translation only. Probably should be removed for consistency?

NicknineTheEagle · 2023-02-10T03:11:15Z

Russian text should say "Образ FAT" instead of "Привод FAT".

kcgen · 2023-02-10T03:11:15Z

Looks good to me 👍🏻 Minor comment: there's a period at the end of the Polish (?) translation only. Probably should be removed for consistency?

I think @FeralChild64 wanted it; but will let him decide.

Co-authored-by: Robert de Rooy <[email protected]> Co-authored-by: FeralChild64 <[email protected]> Co-authored-by: kcgen <[email protected]>

kcgen · 2023-02-10T03:14:18Z

Russian text should say "Образ FAT" instead of "Привод FAT".

added.

GranMinigun · 2023-02-10T07:17:43Z

Ideally the comma before "и" should be dropped (Oxford comma is not a thing in Russian), but I'm nitpicking. I also see that everything is in singular form... gettext, so close yet so far away.

Kappa971 · 2023-02-10T08:07:56Z

Seems ok to me 👍🏻

Burrito78 · 2023-02-10T09:51:36Z

The Oxford comma did not actually originate at Oxford University in England. One can trace its origin in English guides from the early 20th century. The Oxford comma is "correct" in American Standard English but does not exist in other languages, nor is it mandatory in British or International English.
https://libguides.framingham.edu/c.php?g=894986&p=7278665

Also please change "FAT image" -> "FAT-Image" for german.

Thanks, looking good otherwise!

johnnovak · 2023-02-10T11:32:27Z

The Oxford comma did not actually originate at Oxford University in England. One can trace its origin in English guides from the early 20th century. The Oxford comma is "correct" in American Standard English but does not exist in other languages, nor is it mandatory in British or International English.
https://libguides.framingham.edu/c.php?g=894986&p=7278665

Also please change "FAT image" -> "FAT-Image" for german.

Thanks, looking good otherwise!

Can you even call a filesystem FAT these days? Is that politically correct? Shall we rename it to LargeSized?

FeralChild64 · 2023-02-10T16:19:08Z

@kcgen @johnnovak You can remove period at the end of sequence in Polish translation if the original string does not have it. I’m going to check the remaining strings for consistency later.

kcgen · 2023-02-10T16:36:41Z

https://libguides.framingham.edu/c.php?g=894986&p=7278665

Great reference @Burrito78 , especially those examples.

I took an engineering technical writing course (long ago), but the last comma was mandatory to differentiate the last "item" exactly like those examples show (and the confusion and legal troubles it can cause without it!)

This style also makes bullets and commas behave consistently (imagine stuffing the last two items on the same bullet!)

A
B
C
D

kcgen · 2023-02-10T16:54:35Z

@Burrito78

@FeralChild64

I think that does it. Merging. Huge team effort - thanks everyone!

johnnovak · 2023-02-10T22:05:08Z

https://libguides.framingham.edu/c.php?g=894986&p=7278665

Great reference @Burrito78 , especially those examples.

I took an engineering technical writing course (long ago), but the last comma was mandatory to differentiate the last "item" exactly like those examples show (and the confusion and legal troubles it can cause without it!)

This style also makes bullets and commas behave consistently (imagine stuffing the last two items on the same bullet!)
* A

* B

* C

* D

I'm a big fan of the Oxford comma too, and so are some of my Aussie colleagues. As @kcgen said, it removes ambiguity which is always a good thing. In general, I'm a comma maximalist! 😁 I use as much punctuation as I can; I see them as a courtesy to my readers to help them with parsing my sentences more easily.

Burrito78 · 2023-02-13T09:44:09Z

https://www.germanwithnicole.com/blog/36237-sorry-there-is-no-oxford-comma-in-german

Nicole said it best.

So be sure to review your German texts when you've written a list und entfernen Sie das Oxford-Komma!

johnnovak · 2023-02-14T00:55:27Z

On the importance of commas; this is a real example from Hungarian history which is somewhat hard to translate, but it illustrates the concept.

TL;DR the queen was assassinated in the end, and the writer of the message wanted to discourage certain parties from the murder, but it can be actually interpreted both ways.

The original message went like this (without commas):

"Kill Queen you must not fear will be good if all agree I do not oppose"

And the two possible interpretations (the author of the messages claimed the second interpretation was correct, and was hence spared from execution):

"Kill Queen, you must not fear, will be good if all agree, I do not oppose"

"Kill Queen you must not, fear will be good, if all agree I do not, oppose"

kcgen self-assigned this Feb 9, 2023

kcgen added the enhancement New feature or enhancement of existing features label Feb 9, 2023

kcgen requested review from FeralChild64 and johnnovak February 9, 2023 01:37

kcgen changed the title ~~Improve wildcard handling of imgmount file arguments~~ Improve wildcard handling for imgmount file arguments Feb 9, 2023

johnnovak reviewed Feb 9, 2023

View reviewed changes

src/dos/program_imgmount.cpp Outdated Show resolved Hide resolved

johnnovak reviewed Feb 9, 2023

View reviewed changes

src/misc/string_utils.cpp Outdated Show resolved Hide resolved

johnnovak reviewed Feb 9, 2023

View reviewed changes

src/misc/string_utils.cpp Show resolved Hide resolved

johnnovak reviewed Feb 9, 2023

View reviewed changes

include/string_utils.h Show resolved Hide resolved

kcgen force-pushed the kc/improve-wildcard-sort-1 branch from 2694944 to a9e3866 Compare February 9, 2023 18:37

kcgen added 2 commits February 9, 2023 16:59

Sort imgmount's file args when using wildcards

4756ae6

Also tidies up superfluous path specifications to reduce visual clutter in screen and log output.

Improve the logging of disk images when mounting

56f6d60

Includes: - Uses more readable comma-separated format - Adds the filenames to the console log messages

kcgen force-pushed the kc/improve-wildcard-sort-1 branch from a9e3866 to 64c76b5 Compare February 10, 2023 01:43

Format mount status with source followed by drive

4e989e4

Co-authored-by: Robert de Rooy <[email protected]> Co-authored-by: FeralChild64 <[email protected]> Co-authored-by: kcgen <[email protected]>

kcgen force-pushed the kc/improve-wildcard-sort-1 branch from 64c76b5 to 4e989e4 Compare February 10, 2023 03:13

kcgen merged commit d8f58b8 into main Feb 10, 2023

delete-merged-branch bot deleted the kc/improve-wildcard-sort-1 branch February 10, 2023 16:54

kcgen mentioned this pull request Oct 25, 2023

Improve imgmount/boot multiple image format #3045

Closed

2 tasks

Grounded0 mentioned this pull request Nov 17, 2023

DOSBox Staging 0.81 release steering ticket #3091

Closed

4 tasks

johnnovak added the DOS Issues related to DOS integration or DOS commands label Dec 11, 2023

Uh oh!

Improve wildcard handling for imgmount file arguments #2270

Improve wildcard handling for imgmount file arguments #2270

Uh oh!

Conversation

kcgen commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnnovak commented Feb 9, 2023

Uh oh!

kcgen commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rderooy commented Feb 9, 2023

Uh oh!

kcgen commented Feb 9, 2023

Uh oh!

rderooy commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kcgen commented Feb 9, 2023

Uh oh!

kcgen commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rderooy commented Feb 9, 2023

Uh oh!

rderooy commented Feb 9, 2023

Uh oh!

rderooy commented Feb 9, 2023

Uh oh!

FeralChild64 commented Feb 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kcgen commented Feb 9, 2023

Uh oh!

kcgen commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

johnnovak commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicknineTheEagle commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kcgen commented Feb 10, 2023

Uh oh!

kcgen commented Feb 10, 2023

Uh oh!

GranMinigun commented Feb 10, 2023

Uh oh!

Kappa971 commented Feb 10, 2023

Uh oh!

Burrito78 commented Feb 10, 2023

Uh oh!

johnnovak commented Feb 10, 2023

Uh oh!

FeralChild64 commented Feb 10, 2023

Uh oh!

kcgen commented Feb 10, 2023

Uh oh!

kcgen commented Feb 10, 2023

Uh oh!

johnnovak commented Feb 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Burrito78 commented Feb 13, 2023

Uh oh!

johnnovak commented Feb 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Improve wildcard handling for `imgmount` file arguments #2270

Improve wildcard handling for `imgmount` file arguments #2270

kcgen commented Feb 9, 2023 •

edited

Loading

kcgen commented Feb 9, 2023 •

edited

Loading

rderooy commented Feb 9, 2023 •

edited

Loading

kcgen commented Feb 9, 2023 •

edited

Loading

FeralChild64 commented Feb 9, 2023 •

edited

Loading

kcgen commented Feb 10, 2023 •

edited

Loading

johnnovak commented Feb 10, 2023 •

edited

Loading

NicknineTheEagle commented Feb 10, 2023 •

edited

Loading

johnnovak commented Feb 10, 2023 •

edited

Loading

johnnovak commented Feb 14, 2023 •

edited

Loading