Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

Duplicate cask for M+ fonts #2246

Closed
waldyrious opened this issue Aug 1, 2020 · 9 comments · Fixed by #2250
Closed

Duplicate cask for M+ fonts #2246

waldyrious opened this issue Aug 1, 2020 · 9 comments · Fixed by #2250

Comments

@waldyrious
Copy link
Contributor

waldyrious commented Aug 1, 2020

It looks like font-mplus.rb and font-rounded-mplus.rb (introduced in #2178) are duplicates of font-m-plus.rb and font-rounded-m-plus.rb (introduced in #146 and #262, respectively).

Should the former be deleted?

@vitorgalvao
Copy link
Member

Should the former be deleted?

Delete the latter. The former is auto-generated from a script, so it’s more consistent for the future. There may be others that ended up being duplicated like this.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@waldyrious
Copy link
Contributor Author

The manual entry seems to contain a bunch more styles (1c, 1m, 1mn, 1p, 2c, 2m, 2p) than the automatic one (1p only). These correspond to fonts with design differences such as proportional vs. monospaced, etc., so in effect the manual cask does provide a wider variety of fonts than the automatic one. Do we still want to delete it regardless?

@no-response no-response bot removed the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@vitorgalvao
Copy link
Member

The automatic entry will always be recreated when the script is run, so deleting that one is pointless.

To put it bluntly, this repo is a huge time and maintenance drain and isn’t worth the effort, so we need to be efficient. If the script can include those extra versions (are they even in the Google Fonts repo?), so much the better, but that’s the one that needs to stay.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@waldyrious
Copy link
Contributor Author

I'm afraid Google Fonts only includes the 1p version (see here and here).

I'm not sure removing content that isn't otherwise included is a net positive, compared to having a partial duplicate as is currently the case. I'd like to hear the thoughts of @rolandwalker (#146), @emauricio (#1931), @kaorimatz (#433, #551, #915, #1380), and @izumin5210 (#261, #262) and @brian-c (#176), about whether keeping only the 1p variant would be an acceptable outcome. Otherwise we might as well leave things as they are.

On a separate note, maybe @afeld can also comment on what the automated import would do if the cask name already exists (currently they only differ by a hyphen, font-mplus vs font-m-plus). Would the content be rewritten? Would there be an error? It would be nice if the Google Fonts version were skipped, since it's a subset of the other one.

@no-response no-response bot removed the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@vitorgalvao
Copy link
Member

Otherwise we might as well leave things as they are.

Definitely not. We do not want duplicates. This repo is a big enough mess as it is. Everything we can automate is a net win. We’ll not do things that increase the burden of managing this repo.

You’re ignoring the obvious solution that fixes everything for everyone, which is to submit the full font to the Google Fonts repo. HBC shouldn’t be used as a workaround for things missing upstream.

@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@waldyrious
Copy link
Contributor Author

You’re ignoring the obvious solution that fixes everything for everyone, which is to submit the full font to the Google Fonts repo. HBC shouldn’t be used as a workaround for things missing upstream.

I wasn't aware that they accepted community contributions, actually — I thought the fonts there were all manually added and curated by a team at Google, or something. I'll look into adding the additional fonts to their M+ collection, then. I'll post back when I have news. Thanks for the heads-up! 👍

@no-response no-response bot removed the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@vitorgalvao vitorgalvao added the awaiting user reply Issue needs response from a user. label Aug 3, 2020
@afeld
Copy link
Contributor

afeld commented Aug 5, 2020

Since those font folders don't contain a metadata file, the import script derives the font name based on the filename:

# grab the first font, arbitrarily
meta.name = derive_name(font_files[0])

There's no hyphen there, hence no hyphen in the cask. If a cask of the same name already exists and it points somewhere besides Google Fonts, the script will skip it:

def should_skip(cask_path):
if os.path.exists(cask_path):
# Cask already exists
if is_other_foundry(cask_path):
print("Other foundry:", cask_path)
# don't overwrite it, per
# https://github.com/Homebrew/homebrew-cask-fonts/blob/master/CONTRIBUTING.md#google-web-font-directory
return True

Therefore, the simplest solution is to rename the more complete casks to match what's derived from Google: #2250 Does that clarify, and sound reasonable?

@afeld
Copy link
Contributor

afeld commented Aug 5, 2020

Re: adding other MPlus styles to Google Fonts: google/fonts#62 (comment)

@waldyrious
Copy link
Contributor Author

Thanks for the information and for fixing this @afeld! I've offered to help in the issue you linked to; hopefully we may soon revert this cask to the google fonts version :)

@no-response no-response bot removed the awaiting user reply Issue needs response from a user. label Aug 5, 2020
@no-response no-response bot reopened this Aug 5, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants