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

Add new option to keep original glyphs as is when adding single-width icons #1773

Merged
merged 5 commits into from
Jan 8, 2025

Conversation

Finii
Copy link
Collaborator

@Finii Finii commented Dec 31, 2024

Description

At the moment, if you want to have "monospaces" icons you always also force the font to be monospaced in Windows terms.
That has biten us in the past. Maybe It is better to leave that choice to the user and make this two options.

Requirements / Checklist

  • Read the Contributing Guidelines
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.
    Issue number where discussion took place: #xxx
  • If this contains a font/glyph add its origin as background info below (e.g. URL)
  • Verified the license of any newly added font, glyph, or glyph set. License is: xxx

What does this Pull Request (PR) do?

Remove --use-single-width-glyphs (to avoid confusion)
Add --single-with-glyphs which is analogous to --variable-with-glyphs and just affects our added icons.
The old -s and --mono force any font to be strictly monospaced and also sets the new single-with-glyphs option.

How should this be manually tested?

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

@Finii
Copy link
Collaborator Author

Finii commented Jan 1, 2025

From the shellcheck CI

./bin/scripts/archive-fonts.sh:91:10: note: Double quote to prevent globbing and word splitting. [SC2086]

@Finii Finii force-pushed the feature/single-with-glyphs-option branch from b29faeb to 7f7a9fd Compare January 8, 2025 07:09
@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2025

Rebase on master, force push

@Finii Finii marked this pull request as ready for review January 8, 2025 08:15
[why]
There are three possible options to specify the Nerd Font Mono generation:
    -s
    --mono
    --use-single-width-glyphs

All the three are handled the same.

In order to add a new long option that also handles glyph width in a "single"
cell manner the fear is that two too similar options will confuse users.

[how]
Just hide the longest form.
Also remove from the readme files.

Need to adapt the install.sh script.

(The option still works but is not 'advertised'.)

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii force-pushed the feature/single-with-glyphs-option branch from 7fc628b to 171ac54 Compare January 8, 2025 19:42
Finii added 4 commits January 8, 2025 20:43
…uching existing glyphs

[why]
This can help if you want monospaced icons but not-force the other
glyphs to be monospaced (which we do to make the whole font
monospace-detectable which was a major issue in the beginning, esp with
Windows).

[how]
Add option --single-width-glyphs that makes the added glyphs single
width (like --mono), but leaves preexisting glyphs untouched.

Fixes: #1772

Signed-off-by: Fini Jastrow <[email protected]>
[why]
This warning turns up with shellcheck 0.9.0, but not with 0.8.0 which we
used previously.

Signed-off-by: Fini Jastrow <[email protected]>
EXPERIMENTAL

[why]
Sometimes users want to tweak the cell sizing or the baseline to
baseline distance, or the middle point of the cell.

[how]
Add a new option to show the 'cell' box and to override the detected
one. It is not sufficient to adjust width and height, because that can
not define shifts of the cell up/down (left/right is mostly useless and
I believe the code does not work if the xmin is not zero).

The smaller icon-height is not used here (affecting only --mono) because
that seems to compilcated right now.

Signed-off-by: Fini Jastrow <[email protected]>
[why]
I again had to look the reason for the code up, which just looks
strange. Who expects that a bool is an instance of int? I certainly not.

Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii force-pushed the feature/single-with-glyphs-option branch from 171ac54 to 5d7bd1b Compare January 8, 2025 19:46
@Finii
Copy link
Collaborator Author

Finii commented Jan 8, 2025

Also sneaking in the --cell expert option I always wanted to add but never came round to actually do it.
With that after-patching baseline-to-baseline tinkering is not needed anymore and the powerline glyphs match your personal choice.

@Finii Finii merged commit c0f0c70 into master Jan 8, 2025
5 checks passed
@Finii Finii deleted the feature/single-with-glyphs-option branch January 8, 2025 20:45
@Finii Finii mentioned this pull request Jan 14, 2025
4 tasks
@Finii Finii added this to the v3.4.0 milestone Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant