Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can convert foot pounds to Newton meters #92

Closed
Xistof opened this issue Feb 1, 2023 · 12 comments
Closed

Can convert foot pounds to Newton meters #92

Xistof opened this issue Feb 1, 2023 · 12 comments

Comments

@Xistof
Copy link

Xistof commented Feb 1, 2023

I want to convert foot pounds to Newton meters.

I used this syntax in Alfred : 100 ftlb to Nm, and I was expecting the result : 135.582 Nm

The syntax has been well recognized, but I get the following error message : Error, you can't convert ftlb to Nm

I am using the last version of the workflow, all other conversions I tried were ok.

Screenshot

@arvidbjorkstrom
Copy link

My guess is that is because the query it made lowercase by cleanQuery, so the conversion becomes [ftlb] to [nm] and the first match for [nm] is length - nano meters - so according to that the unit types don't match.

Nm is written with a capital N in the unit listing I'm guessing this is done aiming to prevent this, but since the input is lowercased it still doesn't work.

Removing the lowercase conversion will make other unit variations not work (i.e Wh will not match watt hours since its listed as [wh]) so that is not a good way to go I think.

One way to go would be to intelligently try to match them. If the first unit is of the energy category, try to match the second unit within the energy category.
If no matches are found, check what category the second unit is and see if there is a match for the first unit in that category.
That would also solve this issue: #81 (comment)

The downside is that if there are two units that both exist in two categories one of the first matching selection of unit would always happen making it impossible to do the second conversion. But I guess you would just have to look out for that when adding new units.

@arvidbjorkstrom
Copy link

My initial thought wouldn't work without to much rewrites I believe, The units exist in two places - Converter.php and units.php - and they are not coded in groups in Converter.php, so the unit names need to be unique and cannot be overlapping in Converter.php.

A better way might be to keep the unit names unique in units.php, but having duplicate names in the -keys language files. And doing the intelligent matching in getCorrectunitinstead.
What does @biati-digital think? Should I create a pull request with this?

@Xistof
Copy link
Author

Xistof commented Feb 3, 2023

Thank you for looking into it. I did not thought about the unit problem (nano meters) and I think you're completely right with that !
(a better way would be to the US to adopt the metric system.. ikr... )

Concerning removing the lowercase conversion, in my opinion it sounds good : the international system of units uses the capital for units. But I guess for people used to the workflow using lower capitals it's a regression...

@arvidbjorkstrom
Copy link

I kind of agree about removing the lowercase conversion. The starting point should be to try use all of the data the user provided, not start by filtering out information.
The SI unit system uses lower case for all unit symbols except those derived from names - Watt, Newton, Pascal etc. There are a lot of the units derived from names though. =)

One way to still keep backwards compatibility while removing the lowercase conversion could be to extending the -keys files with relevant case variations in combination with an intelligent unit matching feature in the style mentioned above.
The downside is a larger -keys file, but I believe the upside would be a more human friendly behavior.

@biati-digital
Copy link
Owner

Hi, i do not mind removing the lowercase conversion in "cleanQuery" but i think this will require more work becase there are several tools and they all should work the same way and it will require a lot of testing (i wanted to add pestphp for testing a long time ago but i've been really busy). Users should be able to write the query in lowercase, uppercase or both.

@arvidbjorkstrom can you explain a little more why do you think your initial idea would not work?

One way to go would be to intelligently try to match them. If the first unit is of the energy category, try to match the second unit within the energy category.
If no matches are found, check what category the second unit is and see if there is a match for the first unit in that category.

The file Converter.php it's a library that was included at the beginning to save some time but the actual file is not that big, would not be easier to move the code that actually handles the conversion from Converter.php to the units.php file? the Converter.php file includes a some methods that are not used at all like getUnits, removeUnit, addUnit, toAll, toMany, etc...

@arvidbjorkstrom
Copy link

I started coding my first idea, and half way through I realized Converter.php had to be rewritten to take into account the unit category for duplicate unit names to work. That could of course be done (or maybe better to move it into the units file), but then it struck me there is a reason the standardized unit symbols are made to not be overlapping (at least I think they are when including case?).

There is already an alias "front end" in the code to handle alternative names for units with the -keys files. I think the best way would be to continue that route and create code that handles conflicts intelligently and selects the matching standard units for conversion.

[
'nm' => 'Nm', // Newton meters
'nm' => 'nm', // Nano meters
]

@biati-digital
Copy link
Owner

I've fixed this issue, i removed the Converter.php so we have only one file without duplicated units, it was far easier than i thought, now the workflow is able to understand

100 ftlb to Nm
100 Nm to ftlb
100 nm to pm

I'll release the update this week.

@Xistof
Copy link
Author

Xistof commented Mar 15, 2023

Thank you for the fix !!

@biati-digital
Copy link
Owner

There's a new update 4.0.0.

If you are using Alfred 5, this new update uses the new Workflow configuration window, the workflow should be able to grab the previous config and still work correctly but it would be great if you can check the new configuration.

I'm planing to submit the workflow to the Alfred Gallery, but before that i hope someone can help me to try it and let me know if you find bugs or if something can be improved.

@Xistof
Copy link
Author

Xistof commented Mar 20, 2023

Hi; thanks for the update, seems to work fine to convert ftlb to nm.
however, I can't access to the CA command anymore (calculate anything), to check for units, currencies, etc.

@biati-digital
Copy link
Owner

Hi @Xistof yes, the ca command was removed, in order to be able to submit the workflow to the Alfred Gallery, i had to remove the old config command and the updater, sadly the list currencies and list units worked with the old ca command. In the next update i'll include it, for now i've invested a lot of time on this update and i've to get back to work.

If it helps (probably not) you can see the currencies and units here.

List of units
List of currencies

I'm gonna close this issue now as it's resolved.

@Xistof
Copy link
Author

Xistof commented Mar 21, 2023

thank you for the feedback and for this work !! it's perfect !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants