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

X11 icon and other minor changes #377

Merged
merged 4 commits into from
Jun 1, 2016
Merged

Conversation

DamnedScholar
Copy link
Contributor

I've gotten the X11 icon added to Font Mfizz for the purpose of using it with this package. I added extensions for all the .x config files on my personal machine, as well as a couple of extensions for ZSH and Vim files on my local machine that the style didn't include.

@Alhadis Alhadis self-assigned this Jun 1, 2016
@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

winces This isn't gonna be fun to explain...

TL;DR:

Reasons.

Reasons:

I'll start with the easily-digestible:

  1. The package already has a dedicated icon font for custom icon additions.
    The process of adding a new icon is documented in the contribution notes very cleanly. This font exists so we're not left with several ad-hoc icon-fonts floating around. Which used to be the case.

Now for the main course:
2. We're not upgrading Mfizz.
This is why:Figure 1
Look at the codepoints of the icons across the top, and imagine what upgrading the font will do for every user who's using one of its icons in their stylesheet.

That's right, for reasons that fall outside the scope of my comprehension, the Mfizz team decided to completely reindex the font at some point in time, introducing a breaking change that we can't simply integrate without knocking over a few vases. Or several.

Another gripe with Mfizz is the inconsistent size and alignment of each icon:
Figure 2
Devicons is less schizophrenic, but its icons are still uneven and padded with margins:
Figure 3
These discrepancies are the reason for nearly every alignment-related issue that's been reported, and the solution has always been to clobber certain icons with ad-hoc CSS tweaks. Result: 80% satisfaction at the expense of one really messy stylesheet.
3. "Enough whining, what's the reason you're not upgrading?
Because both Mfizz and Devicons are due to be replaced with patched versions in File-Icons v2.0.0.

In acts of questionable sanity, I took it upon myself to manually scale each icon of both fonts and fit them all snugly to ensure consistent sizing. However, most of my time went toward cleaning up excessive geometry to produce sharper details and better filesizes. This actually reduced Devicon's WOFF2 by an insane 17 KBs... and when you consider WOFF2's state-of-the-art compression, 17 KBs is an absurd improvement to be made from simply culling unneeded control points in Adobe Illustrator.
4. "So what's stopping you from adding these fonts now?"
When v2 ships, it'll force users to update customisations in their stylesheets. Since there's a good chance they might've tweaked Devicons and Mfizz's icons themselves, they'll probably need to update such adjustments in their stylesheets as well. We can kill two birds with one stone by only requiring users to update customisations once... simply by delaying the addition of these patched fonts until the next major release.

The new problem:

The package's custom icons font has passed 300 icons, and I'm trying to be conservative with what's added. That is to say, if an icon already exists in a bundled font, it won't be added to the custom icons-font (exceptions were made here to maintain compatibility).

Since Mfixx and DevOpicons will both be updated whenever the originals are updated, I'll naturally have to add the X11 icon to Mfixx and hope to God you'll understand why it'll have to wait until v2 to be added.

@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

So, to bottom-line it for you: you'll have to revert the changes to Mfizz and remove the x11 icon addition, leaving only the "other minor changes". I'll add your x11 icon to MFixx, since it's now part of the font it's derived from.

EDIT: I also noticed the .x extension conflicts with an extension used by [Logos](https://github.com/search?q=extension%3Ax+NOT+nothack&type=Code), an unfortunately-named programming language that seems to be more widespread than shell, if the search result's language statistics are anything to go by.

Are .x config files a common thing? Logos also uses the .xm and .xi extensions, so I suppose we could probably pinch .x from it...

@DamnedScholar
Copy link
Contributor Author

Okay, all of the X11 stuff is reverted for this PR.

*winces* This isn't gonna be fun to explain...

That's an odd reaction.

The process of adding a new icon is documented in the contribution notes very cleanly.

I agree. The first sentence of the contrib notes:

If a desired icon isn't included in one of the existing icon-font packages

I felt like the X11 logo would be a good fit for an icon font that gets used in multiple places. Since it has been accepted to be part of Mfizz, it's included in one of the existing icon-font packages. The contribution notes didn't give me any reason to think that that might be a problem.

That's right, for reasons that fall outside the scope of my comprehension, the Mfizz team decided to completely reindex the font at some point in time, introducing a breaking change that we can't simply integrate without knocking over a few vases.

From what I've seen, the font gets reindexed whenever a new icon gets added. Mine just happened to be last in the alphabet. Mfizz builds with a CSS file so that people only have to use the human-readable names in their own code.

and hope to God you'll understand why it'll have to wait until v2 to be added.

So dramatic. I would have accepted a flat "no", too. I made what I needed for myself. I submitted the PR because X files are on virtually every Unix machine and it was annoying me to see a bunch of files in ~/ sit with blank paper for an icon.

@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

So dramatic. I would have accepted a flat "no", too. I made what I needed for myself.

My exasperation isn't directed at you, so don't misconstrue the tone of my reply as blaming you for an inconvenience. The catalyst of my frustration is mainly helplessness. V2 can't be released until several important PRs are merged upstream, and they've been waiting review for over a month. When you've been itching to shoot two very problematic icons in the head for well over a year, time drags to a crawl.

From what I've seen, the font gets reindexed whenever a new icon gets added. Mine just happened to be last in the alphabet. Mfizz builds with a CSS file so that people only have to use the human-readable names in their own code.

Personally, I feel that's a really myopic approach. What if users prefer to use the font directly, or already have icon classes defined in existing CSS? In any case, Font Awesome gets it right. Icons stay locked into the same codepoint to which they were first assigned, and they don't rely on the assumption users will be using the font in the same way.

That's an odd reaction.

Is it? I'm probably unable to think straight due to not having slept the night before, so I might simply be on-edge or something.

I felt like the X11 logo would be a good fit for an icon font that gets used in multiple places.

That may be true, yes. But the trouble is, most of what gets added to the package's font is probably a similarly good fit:

@Alhadis Alhadis merged commit f0604a5 into file-icons:master Jun 1, 2016
@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

@DamnedScholar Could you please enlighten me as to what language these .x files are written in, and their frequency? I don't think I've happened across them before.

Also, again, I genuinely apologise for my tone, but having to explain this brought up every frustrating hard-to-explain subject I wasn't looking forward to elaborating on.

Alhadis added a commit that referenced this pull request Jun 1, 2016
@DamnedScholar
Copy link
Contributor Author

DamnedScholar commented Jun 1, 2016

I understand. It felt like you expected me to be uncooperative with whatever your decision was, and thus had to preemptively explain everything. That was probably the lack of sleep talking. :)

X11 is the most common windowing system on Unix-like operating systems. Linux users tend to have a bunch of .x<whatever> files in their home folders, which hold the personal configuration files for X and related programs. The frequency is basically only Linux users (and maybe a few OS X users), but virtually every Linux user.

@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

Do these files use a generic INI-like configuration for their syntax, or something domain-specific?

Reason I'm asking is because v2 will introduce the ability to set icons based on manually-assigned grammars, so I'm wondering if I need to take that into consideration as well.

Incidentally, you can find more information on that topic along with the reason for my sleep-deprivation here. My entire night was spent furiously bikeshedding with myself over how best to structure a config file that risks being verbose, inflexible, or unintuitive to new contributors.

Speaking of which, if you've a moment, perhaps give the property descriptions a read and let me know if they make sense. I swear I've spent so long staring at my own code I've lost my ability to recognise incomprehensible bullshit.

@DamnedScholar
Copy link
Contributor Author

The config files in question use generic syntax, different ones use different formats, and they don't have anything cohesive to tie them together (like shebangs or mode lines). So no, don't even consider it. :)

After reading the property descriptions in that file, I feel entirely confident that I could add my own without issue. It reads very well.

@Alhadis
Copy link
Member

Alhadis commented Jun 1, 2016

Alright, good. Just thought I'd check: the bulk of my research was drawn from GitHub's language database, but it's by no means a comprehensive encyclopaedia of every language devised by man.

I feel entirely confident that I could add my own without issue. It reads very well.

SWEET RELIEF. I must've rewritten that six times. Thanks for the feedback! :D

Alhadis added a commit to file-icons/MFixx that referenced this pull request Jun 1, 2016
Alhadis added a commit that referenced this pull request Jun 1, 2016
@Alhadis Alhadis added new-icon New icons added to the package's icon-font. filetype-support New file-extensions or filename patterns associated with an existing icon. labels Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filetype-support New file-extensions or filename patterns associated with an existing icon. new-icon New icons added to the package's icon-font.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants