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

Hasklug ligature issue #934

Closed
akosradler opened this issue Sep 19, 2022 · 6 comments
Closed

Hasklug ligature issue #934

akosradler opened this issue Sep 19, 2022 · 6 comments

Comments

@akosradler
Copy link

🎯 Subject of the issue

Experienced behavior:
Turning ligatures on for Hasklug turns || into a single line |
As these are separate operations in various languages, this behaviour is not desired. (and likely a bug)

🔧 Your Setup

  • Tried with Hasklug Nerd Font Complete Mono Windows Compatible.otf
  • Terminal agnostic
  • Using Windows
@Finii
Copy link
Collaborator

Finii commented Sep 19, 2022

Can reproduce:

image

== ligature works, || jumps to just one (right) bar on entering 2nd char

@Finii
Copy link
Collaborator

Finii commented Sep 19, 2022

Somehow the rule is missing in the Mono font:

image

(E104 is the double-bar glyph)

Ah, yes, it is the old/dysfunctional attempt to remove ligatures from Nerd Font Mono fonts...

image

@Finii
Copy link
Collaborator

Finii commented Sep 22, 2022

The code in font-patcher is wrong.

Original code (commit by Ryan 557fc00 ryanoasis 2016-03-18 23:15 -0400):

# let's deal with ligatures (mostly for monospaced fonts)
if args.configfile and config.read(args.configfile):
  if args.removeligatures:
    print("Removing ligatures from configfile `Subtables` section")
    ligature_subtables = json.loads(config.get("Subtables","ligatures"))
    for subtable in ligature_subtables:
      print("Removing subtable:", subtable)
      try:
        sourceFont.removeLookupSubtable(subtable)
        print("Successfully removed subtable:", subtable)
      except Exception:
        print("Failed to remove subtable:", subtable)
elif args.removeligatures:
  print("Unable to read configfile, unable to remove ligatures")
else:
  print("No configfile given, skipping configfile related actions")

That code has been touched once by Jimmy (commit 0a480bb Jimmy Haas 2018-08-04 03:39 -0700):

def remove_ligatures(self):
    # let's deal with ligatures (mostly for monospaced fonts)
    if self.args.configfile and self.config.read(self.args.configfile):
        if self.args.removeligatures:
            print("Removing ligatures from configfile `Subtables` section")
            ligature_subtables = json.loads(self.config.get("Subtables", "ligatures"))
            for subtable in ligature_subtables:
                print("Removing subtable:", subtable)
            try:
                self.sourceFont.removeLookupSubtable(subtable)
                print("Successfully removed subtable:", subtable)
            except Exception:
                print("Failed to remove subtable:", subtable)
        elif self.args.removeligatures:
            print("Unable to read configfile, unable to remove ligatures")
        else:
            print("No configfile given, skipping configfile related actions")

Note how the indentation changed ... now we have a loop that just prints ... :-(

The bottommost elif and else were also corrupted, but that has already been fixed with c72b0d9 `fracsinus 2021-07-26 06:24 +0900) with #644.

Finii added a commit that referenced this issue Sep 22, 2022
[why]
Only one tables is removed, even if we want to remove more.

With 0a480bb the indentation of the code has changed, and now the loop
is (apart from printing) empty. See also #934

[how]
Re-indent the lines to restore functionality as originally forseen with
commit 557fc00.

Reported-by: Rádler Ákos <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
Finii added a commit that referenced this issue Sep 22, 2022
[why]
Entering two consecutive vertical-bars (i.e. `||`) results in the
display of just one - the right - bar glyph.

[how]
The ligature removal is only partially implemented. The rule at work
here is not removed. There are two parts at work:

One rule to replace the first bar with nothing.

One rule to replace the second bar with a ligarture glyph with negative
left bearing, that shows two bars.

The second rule has been removed, but the first is still there.
This commit also removes the first rule.

[note]
The whole design here is broken. We remove only some rules and leave
others intact, for reasons unknown to me. Other ligartures will also be
only partially removed and leave the user with unreadable output.

We should either remove all (!) tables and not just some, or leave the
ligatures in the font.

As the ligature removal has been broken at large anyhow (see previous
commit), I would suggest to not remove any ligatures (anymore).

Fixes: #934

Reported-by: Rádler Ákos <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii mentioned this issue Sep 22, 2022
2 tasks
Finii added a commit that referenced this issue Sep 22, 2022
[why]
Only one tables is removed, even if we want to remove more.

With 0a480bb the indentation of the code has changed, and now the loop
is (apart from printing) empty. See also #934

[how]
Re-indent the lines to restore functionality as originally forseen with
commit 557fc00.

Reported-by: Rádler Ákos <[email protected]>
Signed-off-by: Fini Jastrow <[email protected]>
@Finii Finii closed this as completed in 12cd703 Sep 22, 2022
@Finii
Copy link
Collaborator

Finii commented Sep 22, 2022

Fixed, thanks for reporting.

@akosradler
Copy link
Author

Thank you for fixing it :)

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues. If you have found a problem that seems similar, please open a new issue, complete the issue template with all the details necessary to reproduce, and mention this issue as reference.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants