diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index 997dd54c..e8d31853 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -149,8 +149,8 @@ jobs: path: | ${{github.workspace}}/build/brewken*.dmg ${{github.workspace}}/build/brewken*.dmg.sha256 - ${{github.workspace}}/mbuild/packages/darwin/Brewken*.dmg - ${{github.workspace}}/mbuild/packages/darwin/Brewken*.dmg.sha256sum + ${{github.workspace}}/mbuild/packages/darwin/brewken*.dmg + ${{github.workspace}}/mbuild/packages/darwin/brewken*.dmg.sha256sum retention-days: 7 - name: Post-processing on any core dump diff --git a/bt b/bt index a472e631..de2cfb49 100755 --- a/bt +++ b/bt @@ -937,7 +937,10 @@ def doSetup(setupOption): print(' cd ' + os.path.relpath(dir_build)) print(' meson compile') print(' meson test') - print(' meson install') + if (platform.system() == 'Linux'): + print(' sudo meson install') + else: + print(' meson install') print(' ' + projectName) @@ -2013,7 +2016,7 @@ def doPackage(): abortOnRunFail(subprocess.run(['ls', '-l', xalanDir], capture_output=False)) # - # Strictly speaking, we should look at every /usr/local/opt/.../*.dylib dependency of our executable, and run + # Strictly speaking, we should look at every /usr/local/opt/../*.dylib dependency of our executable, and run # each of those .dylib files through otool to get its dependencies, then repeat until we find no new # dependencies. Then we should ensure each dependency is copied into the app bundle and whatever depends on it # knows where to find it etc. Pretty soon we'd have ended up reimplementing macdeployqt. Fortunately, in @@ -2055,9 +2058,9 @@ def doPackage(): # dmgbuild (code at https://github.com/dmgbuild/dmgbuild, docs at # https://dmgbuild.readthedocs.io/en/latest/index.html). The advantages of this would be that it would make it # possible to do further fix-up work on the directory tree (if needed) and, potentially, give us more control - # over the disk image (eg to specify an icon for it). However, it's a lot simpler to let macdeployqt create - # the disk image, and we currently don't think we need to do further fix-up work after it's run. A custom icon - # on the disk image would be nice, but is far from essential. + # over the disk image (eg to specify an icon for it). However, it seemed to be fiddly to get it to work. It's + # a lot simpler to let macdeployqt create the disk image, and we currently don't think we need to do further + # fix-up work after it's run. A custom icon on the disk image would be nice, but is far from essential. # log.debug('Running macdeployqt') os.chdir(dir_packages_platform) @@ -2082,71 +2085,40 @@ def doPackage(): abortOnRunFail(subprocess.run(['tree', '-sh'], capture_output=False)) dmgFileName = macBundleDirName.replace('.app', '.dmg') - log.debug('Running otool after macdeployqt') + log.debug('Running otool on ' + capitalisedProjectName + ' executable after macdeployqt') os.chdir(dir_packages_mac_bin) abortOnRunFail(subprocess.run(['otool', '-L', capitalisedProjectName], capture_output=False)) abortOnRunFail(subprocess.run(['otool', '-l', capitalisedProjectName], capture_output=False)) + log.debug('Running otool on ' + xalanLibName + ' library after macdeployqt') + os.chdir(dir_packages_mac_frm) + abortOnRunFail(subprocess.run(['otool', '-L', xalanLibName], capture_output=False)) - # - # TBD: This isn't quite working, so going to try letting macdeployqt make the image - # - # Now that we have the app bundle directory structure, we need to compress it into a dmg file using dmgbuild - # (code at https://github.com/dmgbuild/dmgbuild, docs at https://dmgbuild.readthedocs.io/en/latest/index.html). - # - # You can pass parameters in to dmgbuild on the command line, but it gets a bit unwieldy if you have a lot of - # them. We use the alternative method of providing a configuration file (which is actually just a Python - # script). So we generate that first. - # - # It would be neat if we could invoke dmgbuild directly from Python, but I haven't found a way to do this. - # -# log.debug('Creating .dmg file') -# os.chdir(dir_packages_platform) -# settingsFileName = 'dmgSettings.py' -# dmgFileName = capitalisedProjectName + '-' + buildConfig['CONFIG_VERSION_STRING'] + '.dmg' -# with open(settingsFileName, 'wt') as sf: -# sf.write('#------------------------------------------------------------------------------------\n') -# sf.write('# Settings file for dmgbuild to generate ' + dmgFileName + '\n') -# sf.write('#------------------------------------------------------------------------------------\n') -# sf.write('\n') -# sf.write('# Compressed (gzip) format\n') -# sf.write('format = "UDZO"\n') -# sf.write('\n') -# sf.write('# This is the default file system, but no harm to be explicit\n') -# sf.write('filesystem = "HFS+"\n') -# sf.write('\n') -# sf.write('# Disk image holds just one folder\n') -# sf.write('file = "'+ macBundleDirName + '"\n') -# sf.write('\n') -# sf.write('# Icon used to badge the system’s standard external disk icon\n') -# sf.write('# This is a convenient way to construct a suitable icon from your application\’s icon\n') -# # Doco implies path should start with /Applications/, but this does not work -# sf.write('badge_icon = "'+ macBundleDirName + '/Contents/Resources/' + capitalisedProjectName + 'Icon.icns"\n') -# sf.write('\n') -# sf.write('# Expected usage is drag to install, so default view of icons makes most sense\n') -# sf.write('default_view = "icon-view"\n') -# # -# # Confusingly, although the .dmg file name and the volume name can be passed in via the settings file they are -# # nonetheless required parameters on the command line. Specifically, usage is: -# # dmgbuild [-h] [-s SETTINGS] [-D DEFINES] [--no-hidpi] volume-name output.dmg -# # -# abortOnRunFail( -# subprocess.run( -# ['dmgbuild', -# '-s', settingsFileName, -# capitalisedProjectName + ' ' + buildConfig['CONFIG_VERSION_STRING'], -# dmgFileName], -# capture_output=False -# ) -# ) -# os.chdir(previousWorkingDirectory) log.info('Created ' + dmgFileName + ' in directory ' + dir_packages_platform.as_posix()) - log.debug('Running hdiutil on ' + dmgFileName) + + # + # We can now mount the disk image and check its contents. (I don't think we can modify the contents though.) + # + # By default, a disk image called foobar.dmg will get mounted at /Volumes/foobar. + # + log.debug('Running hdiutil to mount ' + dmgFileName) os.chdir(dir_packages_platform) abortOnRunFail( subprocess.run( ['hdiutil', 'attach', '-verbose', dmgFileName] ) ) + mountPoint = '/Volumes/' + dmgFileName.replace('.dmg', '') + log.debug('Directory tree of disk image') + abortOnRunFail( + subprocess.run(['tree', '-sh', mountPoint], capture_output=False) + ) + log.debug('Running hdiutil to unmount ' + mountPoint) + os.chdir(dir_packages_platform) + abortOnRunFail( + subprocess.run( + ['hdiutil', 'dettach', '-verbose', mountPoint] + ) + ) # # Make the checksum file @@ -2154,6 +2126,8 @@ def doPackage(): log.info('Generating checksum file for ' + dmgFileName) writeSha256sum(dir_packages_platform, dmgFileName) + os.chdir(previousWorkingDirectory) + case _: log.critical('Unrecognised platform: ' + platform.system()) exit(1) diff --git a/meson.build b/meson.build index 0040ce9b..12bb2bf4 100644 --- a/meson.build +++ b/meson.build @@ -93,13 +93,13 @@ # # Then to install: # meson install +# Or, on Linux, you can do: +# sudo meson install +# which avoids the pop-up window # -# Note that, on Linux, although you can do `sudo meson install` to avoids the pop-up window prompting you for a -# password to allow meson to do sudo itself, this is not a good idea. Aside from the general principle of not -# running-as-root for any longer than we need to, the problem is that some files in the mbuild tree (in particular -# meson-logs/install-log.txt) will be created as, and thus owned by, root, with no write access for other users. If you -# then run meson again without sudo, you'll then get an error if it tries to write to file. This certainly happens if -# you run `bt package` for example. +# Note that, on Linux, using `sudo meson install` sometimes creates problems with file permissions - eg +# mbuild/meson-logs/install-log.txt ends up being writable only by root and that results in an error when you run +# `bt package`, but this should be fixed in Meson 1.1.0. # # To build source packages (in the meson-dist subdirectory of the build directory): # meson dist diff --git a/src/BtLineEdit.cpp b/src/BtLineEdit.cpp index 1d151a50..1be80ed2 100644 --- a/src/BtLineEdit.cpp +++ b/src/BtLineEdit.cpp @@ -235,7 +235,7 @@ void BtLineEdit::setText(QString amount, int precision) { Q_FUNC_INFO << "Could not convert" << amount << "(" << this->configSection << ":" << this->editField << ") to double"; } - this->setWidgetText(displayAmount(amt, precision)); + this->setWidgetText(this->displayAmount(amt, precision)); } this->setDisplaySize(force); diff --git a/src/ConverterTool.cpp b/src/ConverterTool.cpp index 2c4a7a4a..8d5bc0af 100644 --- a/src/ConverterTool.cpp +++ b/src/ConverterTool.cpp @@ -1,5 +1,5 @@ /*====================================================================================================================== - * ConverterTool.cpp is part of Brewken, and is copyright the following authors 2009-2021: + * ConverterTool.cpp is part of Brewken, and is copyright the following authors 2009-2023: * • Brian Rower * • Matt Young * • Philip Greggory Lee @@ -27,12 +27,12 @@ ConverterTool::ConverterTool(QWidget* parent) : QDialog(parent) { this->doLayout(); - connect( pushButton_convert, &QAbstractButton::clicked, this, &ConverterTool::convert ); + connect(pushButton_convert, &QAbstractButton::clicked, this, &ConverterTool::convert); return; } void ConverterTool::convert() { - this->outputLineEdit->setText(Measurement::Unit::convert(inputLineEdit->text(), outputUnitsLineEdit->text())); + this->outputLineEdit->setText(Measurement::Unit::convertWithoutContext(inputLineEdit->text(), outputUnitsLineEdit->text())); return; } diff --git a/src/UiAmountWithUnits.h b/src/UiAmountWithUnits.h index 602a0053..11fb21f8 100644 --- a/src/UiAmountWithUnits.h +++ b/src/UiAmountWithUnits.h @@ -46,8 +46,8 @@ struct PreviousScaleInfo { class UiAmountWithUnits { public: /** - * \param - * \param + * \param parent + * \param fieldType * \param units */ UiAmountWithUnits(QWidget * parent, diff --git a/src/measurement/Unit.cpp b/src/measurement/Unit.cpp index aba34156..2fc22df0 100644 --- a/src/measurement/Unit.cpp +++ b/src/measurement/Unit.cpp @@ -19,8 +19,9 @@ =====================================================================================================================*/ #include "measurement/Unit.h" -#include #include +#include // For std::once_flag etc +#include #include #include @@ -48,7 +49,7 @@ namespace { return amtUnit; } - QString unitFromString(QString qstr) { + QString unitNameFromAmountString(QString const & qstr) { QRegExp amtUnit = getRegExpToMatchAmountPlusUnits(); // if the regex dies, return ? @@ -60,7 +61,7 @@ namespace { return amtUnit.cap(2); } - double valueFromString(QString qstr) { + double quantityFromAmountString(QString const & qstr) { QRegExp amtUnit = getRegExpToMatchAmountPlusUnits(); // if the regex dies, return 0.0 @@ -71,21 +72,111 @@ namespace { return Localization::toDouble(amtUnit.cap(1), Q_FUNC_INFO); } - // Note that, although Unit names (ie abbreviations) are unique within a UnitSystem, they are not globally unique. Eg - // "L" is the abbreviation/name of both Liters and Lintner; "gal" is the abbreviation/name of the Imperial gallon and - // the US Customary one; etc. - QMultiMap nameToUnit; + /** + * \brief This is useful to allow us to initialise \c unitNameLookup and \c physicalQuantityToCanonicalUnit after + * all \c Unit and \c UnitSystem objects have been created. + */ + QVector listOfAllUnits; - // NB: There is no canonical unit for Measurement::PhysicalQuantity::Mixed - QMap const physicalQuantityToCanonicalUnit { - {Measurement::PhysicalQuantity::Mass, &Measurement::Units::kilograms}, - {Measurement::PhysicalQuantity::Volume, &Measurement::Units::liters}, - {Measurement::PhysicalQuantity::Time, &Measurement::Units::minutes}, - {Measurement::PhysicalQuantity::Temperature, &Measurement::Units::celsius}, - {Measurement::PhysicalQuantity::Color, &Measurement::Units::srm}, - {Measurement::PhysicalQuantity::Density, &Measurement::Units::sp_grav}, - {Measurement::PhysicalQuantity::DiastaticPower, &Measurement::Units::lintner} + /** + * \brief This allows us to ensure that \c Unit::initialiseLookups is called exactly once + */ + std::once_flag initFlag_Lookups; + + // + // Note that, although Unit names (ie abbreviations) are unique within an individual UnitSystem, some are are not + // globally unique, and some are not even unique within a PhysicalQuantity. For example: + // - "L" is the abbreviation/name of both Liters and Lintner + // - "gal" is the abbreviation/name of the Imperial gallon and the US Customary one + // + // Almost all of the time when we are doing look-ups, we know the PhysicalQuantity (and it is not meaningful for the + // user to specify units relating to a different PhysicalQuantity) so it makes sense to group look-ups by that. + // + struct NameLookupKey { + Measurement::PhysicalQuantity physicalQuantity; + QString lowerCaseUnitName; + // + // We need an ordering on the struct to use it as a key of QMap / QMultiMap. In theory, in C++20, we should be + // able to ask the compiler to do all the work to give a suitable default ordering, by writing: + // + // friend auto operator<=>(NameLookupKey const & lhs, NameLookupKey const & rhs) = default; + // + // However, in practice, this requires the spaceship operator to be defined for QString, which it isn't in Qt5. + // So, for now, define our own operator<, which will suffice for QMultiMap. + // + bool operator<(NameLookupKey const & rhs) const { + NameLookupKey const & lhs = *this; + if (lhs.physicalQuantity != rhs.physicalQuantity) { + return (lhs.physicalQuantity < rhs.physicalQuantity); + } + return (lhs.lowerCaseUnitName < rhs.lowerCaseUnitName); + } }; + QMultiMap unitNameLookup; + + // NB: There is no canonical unit for Measurement::PhysicalQuantity::Mixed + QMap physicalQuantityToCanonicalUnit; + + /** + * \brief Get all units matching a given name and physical quantity + * + * \param name + * \param physicalQuantity + * \param caseInensitiveMatching If \c true, do a case-insensitive search. Eg, match "ml" for milliliters, even + * though the correct name is "mL". This should always be safe to do, as AFAICT there + * are no current or foreseeable units that _we_ use whose names only differ by case -- + * or, at least, that's the case in English... + */ + QList getUnitsByNameAndPhysicalQuantity(QString const & name, + Measurement::PhysicalQuantity const & physicalQuantity, + bool const caseInensitiveMatching) { + // Need this before we reference unitNameLookup or physicalQuantityToCanonicalUnit + std::call_once(initFlag_Lookups, &Measurement::Unit::initialiseLookups); + + auto matches = unitNameLookup.values(NameLookupKey{physicalQuantity, name.toLower()}); + qDebug() << Q_FUNC_INFO << name << "has" << matches.length() << "case-insensitive match(es)"; + if (caseInensitiveMatching) { + return matches; + } + + // If we ever want to do case insensitive matching (which we think should be rare), the simplest thing is just to + // go through all the case-insensitive matches and exclude those that aren't an exact match + decltype(matches) filteredMatches; + for (auto match : matches) { + if (match->name == name) { + filteredMatches.append(match); + } + } + qDebug() << Q_FUNC_INFO << name << "has" << filteredMatches.length() << "case-sensitive match(es)"; + return filteredMatches; + } + + /** + * \brief Get all units matching a given name, but without knowing the physical quantity. Pretty much the only time + * we need this is in \c ConverterTool to do contextless conversions. + * + * \param name + * \param caseInensitiveMatching If \c true, do a case-insensitive search. Eg, match "ml" for milliliters, even + * though the correct name is "mL". + */ + QList getUnitsOnlyByName(QString const & name, + bool const caseInensitiveMatching = true) { + // Need this before we reference unitNameLookup or physicalQuantityToCanonicalUnit + std::call_once(initFlag_Lookups, &Measurement::Unit::initialiseLookups); + + QList allMatches; + for (auto key : unitNameLookup.uniqueKeys()) { + // + // Although it's slightly clunky to call getUnitsByNameAndPhysicalQuantity() here, it saves us re-implementing + // all the case-insensitive logic. + // + auto matches = getUnitsByNameAndPhysicalQuantity(name, key.physicalQuantity, caseInensitiveMatching); + if (matches.length() > 0) { + allMatches.append(matches); + } + } + return allMatches; + } } // This private implementation class holds all private non-virtual members of Unit @@ -99,13 +190,13 @@ class Measurement::Unit::impl { std::function convertToCanonical, std::function convertFromCanonical, double boundaryValue, - Measurement::Unit const & canonical) : + bool isCanonical) : self {self}, unitSystem {unitSystem}, convertToCanonical {convertToCanonical}, convertFromCanonical{convertFromCanonical}, boundaryValue {boundaryValue}, - canonical {canonical} { + isCanonical {isCanonical} { return; } @@ -121,10 +212,9 @@ class Measurement::Unit::impl { std::function convertToCanonical; std::function convertFromCanonical; double boundaryValue; - Measurement::Unit const & canonical; //,:TODO:. Get rid of this + bool isCanonical; }; - Measurement::Unit::Unit(UnitSystem const & unitSystem, QString const unitName, std::function convertToCanonical, @@ -137,13 +227,32 @@ Measurement::Unit::Unit(UnitSystem const & unitSystem, convertToCanonical, convertFromCanonical, boundaryValue, - canonical ? *canonical : *this)} { - nameToUnit.insert(this->name, this); + !canonical /* canonical == nullptr means isCanonical! */ )} { + // + // You might think here would be a neat place to the Unit we are constructing to unitNameLookup and, if appropriate, + // physicalQuantityToCanonicalUnit. However, there is not guarantee that unitSystem is constructed at this point, so + // unitSystem.getPhysicalQuantity() could result in a core dump. + // + // What we can do safely is add ourselves to listOfAllUnits + // + listOfAllUnits.append(this); return; } Measurement::Unit::~Unit() = default; +void Measurement::Unit::initialiseLookups() { + for (auto const unit : listOfAllUnits) { + Measurement::PhysicalQuantity const physicalQuantity = unit->pimpl->unitSystem.getPhysicalQuantity(); + unitNameLookup.insert(NameLookupKey{physicalQuantity, unit->name.toLower()}, unit); + if (!unit->pimpl->isCanonical) { + physicalQuantityToCanonicalUnit.insert(physicalQuantity, unit); + } + } + return; +} + + bool Measurement::Unit::operator==(Unit const & other) const { // Since we're not intending to create multiple instances of any given UnitSystem, it should be enough to check // the addresses are equal, but, as belt-and-braces, we'll check the names & physical quantities are equal as a @@ -178,6 +287,9 @@ double Measurement::Unit::boundary() const { } Measurement::Unit const & Measurement::Unit::getCanonicalUnit(Measurement::PhysicalQuantity const physicalQuantity) { + // Need this before we reference unitNameLookup or physicalQuantityToCanonicalUnit + std::call_once(initFlag_Lookups, &Measurement::Unit::initialiseLookups); + // It's a coding error if there is no canonical unit for a real physical quantity (ie not Mixed). (And of course // there should be no Unit or UnitSystem for Mixed.) Q_ASSERT(physicalQuantityToCanonicalUnit.contains(physicalQuantity)); @@ -185,49 +297,56 @@ Measurement::Unit const & Measurement::Unit::getCanonicalUnit(Measurement::Physi return *canonical; } -QString Measurement::Unit::convert(QString qstr, QString toUnit) { - - QString fName = unitFromString(qstr); - Unit const * f = Measurement::Unit::getUnit(fName); - - double si; - if (f) { - double amt = valueFromString(qstr); - si = f->toCanonical(amt).quantity(); - } else { - si = 0.0; - } - - Unit const * u = Measurement::Unit::getUnit(toUnit); - - // If we couldn't find either unit, or the two units don't match (eg, you - // cannot convert L to lb) - if (u == nullptr || f == nullptr || u->getPhysicalQuantity() != f->getPhysicalQuantity()) { - return QString("%1 ?").arg(Measurement::displayQuantity(si, 3)); +QString Measurement::Unit::convertWithoutContext(QString const & qstr, QString const & toUnitName) { + + qDebug() << Q_FUNC_INFO << "Trying to convert" << qstr << "to" << toUnitName; + double fromQuantity = quantityFromAmountString(qstr); + + QString const fromUnitName = unitNameFromAmountString(qstr); + auto const fromUnits = getUnitsOnlyByName(fromUnitName); + auto const toUnits = getUnitsOnlyByName(toUnitName); + qDebug() << + Q_FUNC_INFO << "Found" << fromUnits.length() << "matches for" << fromUnitName << "and" << toUnits.length() << + "matches for" << toUnitName; + + if (fromUnits.length() > 0 && toUnits.length() > 0) { + // We found at least one match for both "from" and "to" unit names. We need to check search amongst these to find + // one where both units relate to the same physical quantity. + for (auto const fromUnit : fromUnits) { + for (auto const toUnit : toUnits) { + // Stop at the first match. If there's more than one match then we don't have a means to disambiguate. + if (fromUnit->getPhysicalQuantity() == toUnit->getPhysicalQuantity()) { + double canonicalQuantity = fromUnit->toCanonical(fromQuantity).quantity(); + double toQuantity = toUnit->fromCanonical(canonicalQuantity); + return QString("%1 %2").arg(Measurement::displayQuantity(toQuantity, 3)).arg(toUnit->name); + } + } + } } - return QString("%1 %2").arg(Measurement::displayQuantity(u->fromCanonical(si), 3)).arg(toUnit); + // If we didn't recognise from or to units, or we couldn't find a pair for the same PhysicalQuantity, the we return + // the original amount with a question mark. + return QString("%1 ?").arg(Measurement::displayQuantity(fromQuantity, 3)); } Measurement::Unit const * Measurement::Unit::getUnit(QString const & name, - std::optional physicalQuantity) { - auto const numMatches = nameToUnit.count(name); + Measurement::PhysicalQuantity const & physicalQuantity, + bool const caseInensitiveMatching) { + auto matches = getUnitsByNameAndPhysicalQuantity(name, physicalQuantity, caseInensitiveMatching); + + auto const numMatches = matches.length(); if (0 == numMatches) { - qDebug() << Q_FUNC_INFO << name << "does not match any Unit"; return nullptr; } - qDebug() << Q_FUNC_INFO << name << "has" << numMatches << "match(es)"; - // Under most circumstances, there is a one-to-one relationship between unit string and Unit. C will only map to // Measurement::Unit::Celsius, for example. If there's only one match, just return it. if (1 == numMatches) { - Measurement::Unit const * unitToReturn = nameToUnit.value(name); - if (physicalQuantity && unitToReturn->getPhysicalQuantity() != *physicalQuantity) { + auto unitToReturn = matches.at(0); + if (unitToReturn->getPhysicalQuantity() != physicalQuantity) { qWarning() << - Q_FUNC_INFO << "Unit" << name << "matches a unit of type" << - Measurement::getDisplayName(unitToReturn->getPhysicalQuantity()) << "but caller specified" << - Measurement::getDisplayName(*physicalQuantity); + Q_FUNC_INFO << "Unit" << name << "matches a unit of type" << unitToReturn->getPhysicalQuantity() << + "but caller specified" << physicalQuantity; return nullptr; } return unitToReturn; @@ -237,16 +356,15 @@ Measurement::Unit const * Measurement::Unit::getUnit(QString const & name, // Loop through the found Units, like Measurement::Unit::us_quart and // Measurement::Unit::imperial_quart, and try to find one that matches the global default. Measurement::Unit const * defUnit = nullptr; - for (auto ii = nameToUnit.find(name); ii != nameToUnit.end() && ii.key() == name; ++ii) { - Measurement::Unit const * unit = ii.value(); + for (auto const unit : matches) { auto const & displayUnitSystem = Measurement::getDisplayUnitSystem(unit->getPhysicalQuantity()); qDebug() << Q_FUNC_INFO << "Look at" << *unit << "from" << unit->getUnitSystem() << "(Display Unit System for" << unit->getPhysicalQuantity() << "is" << displayUnitSystem << ")"; - if (physicalQuantity && unit->getPhysicalQuantity() != *physicalQuantity) { + if (unit->getPhysicalQuantity() != physicalQuantity) { // If the caller knows the amount is, say, a Volume, don't bother trying to match against units for any other // physical quantity. - qDebug() << Q_FUNC_INFO << "Ignoring match in" << unit->getPhysicalQuantity() << "as not" << *physicalQuantity; + qDebug() << Q_FUNC_INFO << "Ignoring match in" << unit->getPhysicalQuantity() << "as not" << physicalQuantity; continue; } @@ -265,14 +383,46 @@ Measurement::Unit const * Measurement::Unit::getUnit(QString const & name, return defUnit; } +Measurement::Unit const * Measurement::Unit::getUnit(QString const & name, + Measurement::UnitSystem const & unitSystem, + bool const caseInensitiveMatching) { + auto matches = getUnitsByNameAndPhysicalQuantity(name, unitSystem.getPhysicalQuantity(), caseInensitiveMatching); + + // + // At this point, matches is a list of all units matching the supplied name and the PhysicalQuantity of the supplied + // UnitSystem. If we have more than one match, then we prefer the first one we find (if any) in the supplied + // UnitSystem, otherwise, first in the list will have to do. + // + + auto const numMatches = matches.length(); + if (0 == numMatches) { + return nullptr; + } + + if (1 == numMatches) { + return matches.at(0); + } + + for (auto const match : matches) { + if (match->getUnitSystem() == unitSystem) { + return match; + } + } + + return matches.at(0); +} // This is where we actually define all the different units and how to convert them to/from their canonical equivalents // Previously this was done with a huge number of subclasses, but lambdas mean that's no longer necessary // Note that we always need to define the canonical Unit for a given PhysicalQuantity before any others // namespace Measurement::Units { - //: NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be - // unique for that type of unit. Eg you cannot have two units of weight with the same abbreviated name. + // :NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be + // unique for that type of unit. Eg you cannot have two units of weight with the same + // abbreviated name. Ideally, this should also be true on a case insensitive basis, eg it is + // undesirable for "foo" and "Foo" to be the abbreviated names of two different units of + // the same type. + // // === Mass === // See comment in measurement/UnitSystem.cpp for why we have separate entities for US Customary pounds/ounces and Imperials ones, even though they are, in fact, the same diff --git a/src/measurement/Unit.h b/src/measurement/Unit.h index 7aa55cd7..ca3237b0 100644 --- a/src/measurement/Unit.h +++ b/src/measurement/Unit.h @@ -77,6 +77,18 @@ namespace Measurement { ~Unit(); + /** + * \brief This gets called by \c getUnit, \c getCanonicalUnit and \c convertWithoutContext to ensure their lookup + * maps are initialised. + * + * It would be private, except we need to call it from an anonymous namespace function in + * \c measurement/Unit.cpp. + * + * It would be an anonymous namespace function itself, except it needs access to private members of + * \c Unit. + */ + static void initialiseLookups(); + /** * \brief Test whether two \c Unit references are the same. (This is by no means a full test for equality, * since we assume there is only one, constant, instance of each different \c Unit. @@ -133,14 +145,36 @@ namespace Measurement { * know what "qt" is, we go searching for it. * * \param name - * \param physicalQuantity If the caller knows what \c PhysicalQuantity the name relates to, this will help with - * disambiguation (eg between Liters and Lintner, both of which have name/abbreviation - * "L"). Otherwise specify \c std::nullopt here. + * \param physicalQuantity Caller supplies this to help with disambiguation (eg between Liters and Lintner, both + * of which have name/abbreviation "L"). + * \param caseInensitiveMatching If \c true (the default), this means we'll do a case-insensitive search. Eg, + * we'll match "ml" for milliliters, even though the correct name is "mL". This + * should always be safe to do, as AFAICT there are no current or foreseeable units + * that _we_ use whose names only differ by case. + * + * \return \c nullptr if no sane match could be found + */ + static Unit const * getUnit(QString const & name, + Measurement::PhysicalQuantity const & physicalQuantity, + bool const caseInensitiveMatching = true); + + /** + * \brief Try to find a Unit by name in the supplied UnitSystem. If no unit is found, search against the + * PhysicalQuantity to which the supplied UnitSystem relates (which is doable because, per the comment in + * measurement/UnitSystem.h, we each UnitSystem relates to a single PhysicalQuantity). + * + * \param name + * \param unitSystem + * \param caseInensitiveMatching If \c true (the default), this means we'll do a case-insensitive search. Eg, + * we'll match "ml" for milliliters, even though the correct name is "mL". This + * should always be safe to do, as AFAICT there are no current or foreseeable units + * that _we_ use whose names only differ by case. * * \return \c nullptr if no sane match could be found */ static Unit const * getUnit(QString const & name, - std::optional physicalQuantity = std::nullopt); + Measurement::UnitSystem const & unitSystem, + bool const caseInensitiveMatching = true); /** * \brief Get the canonical \c Unit for a given \c PhysicalQuantity. This will be the unit we use for storing @@ -156,7 +190,7 @@ namespace Measurement { * we are dealing with because it's a generic tool to allow the user to convert "3 qt" to liters or "5lb" * to kilograms etc. */ - static QString convert(QString qstr, QString toUnit); + static QString convertWithoutContext(QString const & qstr, QString const & toUnitName); private: // Private implementation details - see https://herbsutter.com/gotw/_100/ diff --git a/src/measurement/UnitSystem.cpp b/src/measurement/UnitSystem.cpp index 808215c7..3f725ede 100644 --- a/src/measurement/UnitSystem.cpp +++ b/src/measurement/UnitSystem.cpp @@ -87,21 +87,43 @@ class Measurement::UnitSystem::impl { return this->scaleToUnit.value(relativeScale, nullptr); } - /** - * \brief Maps from unit name (in this \c UnitSystem) to \c Unit - */ - Unit const * getUnitFromName(QString const & name) const { - auto const unitsForThisSystem = this->scaleToUnit.values(); - auto matchingUnit = std::find_if( - unitsForThisSystem.begin(), - unitsForThisSystem.end(), - [name](Measurement::Unit const * unit) {return unit->name == name;} - ); - if (matchingUnit != unitsForThisSystem.end()) { - return *matchingUnit; - } - return nullptr; - } +/// /** +/// * \brief Maps from unit name (in this \c UnitSystem) to \c Unit +/// * +/// * \param name +/// * \param caseInensitiveFallback If \c true (the default), this means we'll do a case-insensitive search if we didn't +/// * find \c name as a case-sensitive match. Eg, we'll match "ml" for milliliters, even +/// * though the correct name is "mL". This should always be safe to do, as AFAICT there +/// * are no current or foreseeable units that _we_ use whose names only differ by case. +/// */ +/// Unit const * getUnitFromName(QString const & name, bool const caseInensitiveFallback = true) const { +/// auto const unitsForThisSystem = this->scaleToUnit.values(); +/// auto matchingUnit = std::find_if( +/// unitsForThisSystem.begin(), +/// unitsForThisSystem.end(), +/// [name](Measurement::Unit const * unit) {return unit->name == name;} +/// ); +/// +/// // +/// // If we didn't find an exact match, we'll try a case-insensitive one if so-configured. (We don't do this by +/// // default as (a) the assumption is that it's rare we'll need the case insensitivity, and (b) if we ever did have +/// // an instance where case sensitivity were important - eg "cal" and "Cal" - then it would be incorrect to do case +/// // insensitive matching first.) +/// // +/// if (matchingUnit != unitsForThisSystem.end() && caseInensitiveFallback) { +/// matchingUnit = std::find_if( +/// unitsForThisSystem.begin(), +/// unitsForThisSystem.end(), +/// [name](Measurement::Unit const * unit) {return unit->name.toLower() == name.toLower();} +/// ); +/// } +/// +/// +/// if (matchingUnit != unitsForThisSystem.end()) { +/// return *matchingUnit; +/// } +/// return nullptr; +/// } /** * \brief This does most of the work for displayAmount() and amountDisplay() @@ -224,12 +246,10 @@ Measurement::Amount Measurement::UnitSystem::qstringToSI(QString qstr, Unit cons Unit const * unitToUse = nullptr; if (!unitName.isEmpty()) { - // The supplied string specifies units, so see if they are ones we recognise in this unit system - unitToUse = this->pimpl->getUnitFromName(unitName); - // If we didn't find the specified units in this UnitSystem, broaden the search and look in all units - if (!unitToUse) { - unitToUse = Measurement::Unit::getUnit(unitName, this->pimpl->physicalQuantity); - } + // Unit::getUnit() will, by preference, match to a unit in the current UnitSystem if possible. If not, it will + // match to a unit in another UnitSystem for the same PhysicalQuantity. If there are no matches that way, it will + // return nullptr; + unitToUse = Unit::getUnit(unitName, *this, true); if (unitToUse) { qDebug() << Q_FUNC_INFO << this->uniqueName << ":" << unitName << "interpreted as" << unitToUse->name; } else { @@ -245,8 +265,8 @@ Measurement::Amount Measurement::UnitSystem::qstringToSI(QString qstr, Unit cons Measurement::Amount siAmount = unitToUse->toCanonical(amt); qDebug() << - Q_FUNC_INFO << this->uniqueName << ": " << qstr << "is" << amt << " " << unitToUse->name << "=" << siAmount.quantity() << - "in" << siAmount.unit()->name; + Q_FUNC_INFO << this->uniqueName << ": " << qstr << "is" << amt << " " << unitToUse->name << "=" << + siAmount.quantity() << "in" << siAmount.unit()->name; return siAmount; } diff --git a/src/utils/EnumStringMapping.cpp b/src/utils/EnumStringMapping.cpp index efa7f8b0..f614f230 100644 --- a/src/utils/EnumStringMapping.cpp +++ b/src/utils/EnumStringMapping.cpp @@ -30,7 +30,7 @@ std::optional EnumStringMapping::stringToEnumAsInt(QString const & stringVa [stringValue](EnumAndItsString const & ii){return stringValue == ii.string;}); // // If we didn't find an exact match, we'll try a case-insensitive one if so-configured. (We don't do this by - // default as the assumption is that it's rare we'll need the case insensitivity. + // default as the assumption is that it's rare we'll need the case insensitivity.) // if (match == this->end() && caseInensitiveFallback) { match = std::find_if( diff --git a/translations/bt_ca.ts b/translations/bt_ca.ts index 78573d97..52760756 100644 --- a/translations/bt_ca.ts +++ b/translations/bt_ca.ts @@ -3080,7 +3080,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_cs.ts b/translations/bt_cs.ts index abb65dfc..63aeeb6f 100644 --- a/translations/bt_cs.ts +++ b/translations/bt_cs.ts @@ -3012,7 +3012,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_de.ts b/translations/bt_de.ts index 94cbff56..7917dd9c 100644 --- a/translations/bt_de.ts +++ b/translations/bt_de.ts @@ -3048,7 +3048,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_el.ts b/translations/bt_el.ts index 10c23621..ae0d68ae 100644 --- a/translations/bt_el.ts +++ b/translations/bt_el.ts @@ -3016,7 +3016,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_en.ts b/translations/bt_en.ts index 6ea8804a..8c81fdef 100644 --- a/translations/bt_en.ts +++ b/translations/bt_en.ts @@ -2404,7 +2404,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_es.ts b/translations/bt_es.ts index 2a022d47..58681d64 100644 --- a/translations/bt_es.ts +++ b/translations/bt_es.ts @@ -3068,7 +3068,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_et.ts b/translations/bt_et.ts index 3184cd93..0e4b76d0 100644 --- a/translations/bt_et.ts +++ b/translations/bt_et.ts @@ -2455,7 +2455,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_eu.ts b/translations/bt_eu.ts index f9112551..2174f297 100644 --- a/translations/bt_eu.ts +++ b/translations/bt_eu.ts @@ -2463,7 +2463,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_fr.ts b/translations/bt_fr.ts index fc749fc4..7887ca53 100644 --- a/translations/bt_fr.ts +++ b/translations/bt_fr.ts @@ -3084,7 +3084,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_gl.ts b/translations/bt_gl.ts index f4161e56..a006e481 100644 --- a/translations/bt_gl.ts +++ b/translations/bt_gl.ts @@ -2578,7 +2578,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_hu.ts b/translations/bt_hu.ts index e8cb581f..e05d06c5 100644 --- a/translations/bt_hu.ts +++ b/translations/bt_hu.ts @@ -3056,7 +3056,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_it.ts b/translations/bt_it.ts index 156a480f..3edb6000 100644 --- a/translations/bt_it.ts +++ b/translations/bt_it.ts @@ -3084,7 +3084,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_lv.ts b/translations/bt_lv.ts index dfecdc1b..ed72b23e 100644 --- a/translations/bt_lv.ts +++ b/translations/bt_lv.ts @@ -2518,7 +2518,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_nb.ts b/translations/bt_nb.ts index 7b0602a2..0c2ee7aa 100644 --- a/translations/bt_nb.ts +++ b/translations/bt_nb.ts @@ -3012,7 +3012,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_nl.ts b/translations/bt_nl.ts index f542fe48..a917008d 100644 --- a/translations/bt_nl.ts +++ b/translations/bt_nl.ts @@ -3082,7 +3082,6 @@ Press OK to quit. kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_pl.ts b/translations/bt_pl.ts index ea157d37..6a2bcd0a 100644 --- a/translations/bt_pl.ts +++ b/translations/bt_pl.ts @@ -3012,7 +3012,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_pt.ts b/translations/bt_pt.ts index 7895f063..f9783471 100644 --- a/translations/bt_pt.ts +++ b/translations/bt_pt.ts @@ -3032,7 +3032,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_ru.ts b/translations/bt_ru.ts index 3b14b645..9fb98379 100644 --- a/translations/bt_ru.ts +++ b/translations/bt_ru.ts @@ -3064,7 +3064,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_sr.ts b/translations/bt_sr.ts index 79c3e6c1..4c820eb9 100644 --- a/translations/bt_sr.ts +++ b/translations/bt_sr.ts @@ -2892,7 +2892,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_sv.ts b/translations/bt_sv.ts index edef8570..f3766932 100644 --- a/translations/bt_sv.ts +++ b/translations/bt_sv.ts @@ -3052,7 +3052,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_tr.ts b/translations/bt_tr.ts index 96c59080..d5683615 100644 --- a/translations/bt_tr.ts +++ b/translations/bt_tr.ts @@ -2459,7 +2459,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be diff --git a/translations/bt_zh.ts b/translations/bt_zh.ts index 34ae5ce3..f068f672 100644 --- a/translations/bt_zh.ts +++ b/translations/bt_zh.ts @@ -2980,7 +2980,6 @@ Error message: kg - NOTE FOR TRANSLATORS: The abbreviated name of each unit (eg "kg" for kilograms, "g" for grams, etc) must be