wiedehopf / tar1090

Provides an improved webinterface for use with ADS-B decoders readsb / dump1090-fa
Other
1.22k stars 224 forks source link

Fix many formatting issues #173

Closed getsnoopy closed 2 years ago

getsnoopy commented 2 years ago

This commit fixes many issues where spaces are used incorrectly or unit symbols are used incorrectly in order to aid readability, make the formatting universal, and to reduce confusion.

wiedehopf commented 2 years ago

Applied the rest of the changes manually.

getsnoopy commented 2 years ago

@wiedehopf If you're trying to avoid line breaks, why not properly use NBSP instead of reverting the space, which is just wrong and looks bad?

As for kts, that's just incorrect; it would mean "kilotonne-seconds", which is obviously not what is meant. The SI and larger metric system use symbols (not abbreviations; i.e., they are not pluralized, capitalized unnecessarily, etc.) and the internationally standard symbol for the knot is kn.

Same thing with lbs; it's just incorrect, as it means "pound-seconds".

I wished you'd have had the discussion before merging one way or another. Anyway, I hope you can fix this stuff; otherwise, it just looks bad.

wiedehopf commented 2 years ago

kilotonne-seconds would be kt s with a space, similar for lb s.

Changed the headers to have spaces. Anyhow i don't care about lbs vs lb ... so here you go.

In regards to kts ...

kt is also common, especially in aviation, where it is the form recommended by the International Civil Aviation Organization (ICAO).

Let's go back to kt, i think that's what it was before this discussion.

getsnoopy commented 2 years ago

kilotonne-seconds would be kt s with a space, similar for lb s.

As would kilowatt-hour be kW⋅h or kW h instead of "kWh", but the latter is what people use, unfortunately ;)

I also had a fix in the library file to fix the map legend to read nmi instead of nm (nanometres).

kn is the official one everywhere else, including the IMO, and it avoids the confusion with "kilotonne". I'm getting the ICAO to change their recommendation. I really wish you'd have merged the commit in wholesale, as it had a bunch of other fixes.

wiedehopf commented 2 years ago

Checked again, found 5 little things to apply. NNBSP instead of NBSP is on purpose and will stay. Everything else should be applied.

As for the library: https://github.com/openlayers/openlayers

Is it fixed on their github? Then i can rebuild the library bundle i use.

getsnoopy commented 2 years ago

What is that purpose? For variable width fonts, the difference between NNBSP and NBSP rarely makes enough of a difference that would justify using one over the other.

I haven't fixed it in the library yet. I wanted to apply the fix here as well, since it looks like a custom build of that library that was committed directly into the repo rather than using NPM or the like to build it in, and this would get the change in faster than whenever the change would get in on their side. Once it's fixed there, it will trickle down to here as well eventually.

wiedehopf commented 2 years ago

Updated openlayers, changed the nm to nmi in one spot, it seems to do the trick for the ruler.

Now before you put any more effort in patching cosmetics on this project: don't. My willingness to deal with cosmetic changest that don't actually improve functionality has limits and i'm pretty much there.