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

Fix many formatting issues #173

Closed
wants to merge 1 commit into from

Conversation

getsnoopy
Copy link
Contributor

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.

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 added a commit that referenced this pull request Apr 9, 2022
implement some of the changes in #173 by getsnoopy
@wiedehopf
Copy link
Owner

  • Some of the spacing choices (NNBSP for example).
  • Column headers are lacking spaces to avoid line breaks in the header. (yes it's a hack but it works)
  • kts is a common an perfectly readable abbreviation for knots (used on other tracking sites as well, i don't see a reason to change this)

Applied the rest of the changes manually.

@wiedehopf wiedehopf closed this Apr 9, 2022
wiedehopf added a commit that referenced this pull request Apr 9, 2022
implement some of the changes in #173 by getsnoopy
@getsnoopy
Copy link
Contributor Author

getsnoopy commented Apr 9, 2022

@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
Copy link
Owner

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
Copy link
Contributor Author

getsnoopy commented Apr 9, 2022

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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.

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

Successfully merging this pull request may close these issues.

None yet

2 participants