Skip to content

Enhance OpenType feature code editor 2nd attempt - #2212

Merged
justvanrossum merged 3 commits into
fontra:mainfrom
navv-1:opentype-feature-editor-2
Jul 8, 2025
Merged

Enhance OpenType feature code editor 2nd attempt#2212
justvanrossum merged 3 commits into
fontra:mainfrom
navv-1:opentype-feature-editor-2

Conversation

@navv-1

@navv-1 navv-1 commented Jul 5, 2025

Copy link
Copy Markdown
Contributor

Fixes #2186, fixes #2101, fixes #2106 and add indent with tab support

For color coding/syntax highlighting is using CodeMirror 5-style stream parser which may need to be upgraded for more advanced needs. But I think it works for now.

Undo/redo and toggle comment shortcuts still use codemirror keymaps.

navv-1 added 2 commits July 6, 2025 02:27
… OpenType feature code panel with syntax highlighting, improve styling, and add indentWithTab support
@navv-1

navv-1 commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

The basicSetup includes historyKeymap that handles undo/redo and undoSelection/redoSelection. So it can't be used if we want to use the Fontra shortcuts system, right?

@justvanrossum

justvanrossum commented Jul 7, 2025

Copy link
Copy Markdown
Member

I'm afraid I'm seeing the same behavior as with the previous PR:

  • In Chrome, the tab hangs, I cannot switch to other font info panels
  • In Firefox, mostlty the same as in Chrome
  • In Safari it works

@navv-1

navv-1 commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

Hmm, I'm on Windows; I try it in Chrome and Firefox, and it looks fine.

@justvanrossum

justvanrossum commented Jul 7, 2025

Copy link
Copy Markdown
Member

Ahh, it has to do with the .fea data that I'm looking at, esp. messed up data.

Here's a snippet that manages to make the feature editor/panel hang (at least in Chrome and Ff, yet interestingly not in Safari):

languagesystem latn GRN;
languagesystem latn AFK;
@FRACTIONS = [onehalf onethird onequarter onefifth onesixth oneseventh oneeighth oneninth twothirds twofifths twosevenths twoninths threequarters threefifths threesevenths threeeighths fourfifths foursevenths fourninths fivesixths fivesevenths fiveeighths fiveninths sixsevenths seveneighths sevenninths eightninths];
@ONE = [one one.tlf one.posf onesuperior oneinferior one.numerator one.denominator];
@TWO = [two two.tlf two.posf twosuperior twoinferior two.numerator two.denominator];
@THREE = [three three.tlf three.posf threesuperior threeinferior three.numerator three.denominator];
@FOUR = [four four.tlf four.posf foursuperior fourinferior four.numerator four.denominator];
@FIVE = [five five.tlf five.posf fivesuperior fiveinferi


ddd

or five.numerator five.denominator];
@SIX = [six six.tlf six.posf sixsuperior sixinferior six.numerator six.denominator];
@SEVEN = [seven seven.tlf seven.posf sevensuperior seveninferior seven.numerator seven.denominator];
@EIGHT = [eight eight.tlf eight.posf eightsuperior eightinferior eight.numerator eight.denominator];
@NINE = [nine nine.tlf nine.posf ninesuperior nineinferior nine.numerator nine.denominator];

I hope you can reproduce it on Windows in Chrome or Firefox.

@navv-1

navv-1 commented Jul 7, 2025

Copy link
Copy Markdown
Contributor Author

You're right. The glyph class regex that causes the hang. That can't handle this part:

@FIVE = [five five.tlf five.posf fivesuperior fiveinferi


ddd

or five.numerator five.denominator];

@justvanrossum

Copy link
Copy Markdown
Member

Can the regexes be written in a way so they can never hang? The syntax coloring obviously needs to be resilient against invalid input.

…ighlighting support (previously combined with glyph class)
@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

It should work now

@justvanrossum

Copy link
Copy Markdown
Member

Thank you, it is working really well now.

I've been looking into why integration with the Fontra shortcuts doesn't work, and there are two reasons:

  • a bug/oversight in the panel base code (fixed in the meantime)
  • it on purpose skips shortcuts in editable text fields, and the code editor counts as one

So, for now, this is just not expected to work, and let's keep the codemirror keymap as it is.

I'm ready to merge this. Are you ready with it? I see you added many more nice things in the meantime, really great!

@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

I missed the spec that ranges, glyphs, and glyph class names can be combined in a glyph class like this:

[A.oldstyle - Z.oldstyle ampersand.oldstyle  @smallCaps]

[@Vowels.lc @Vowels.uc y Y]

The coloring will be messed up in this case.

@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

I missed the spec that ranges, glyphs, and glyph class names can be combined in a glyph class like this:

[A.oldstyle - Z.oldstyle ampersand.oldstyle  @smallCaps]

[@Vowels.lc @Vowels.uc y Y]

The coloring will be messed up in this case.

I will try to fix this issue in next PR

@justvanrossum
justvanrossum merged commit 8ad2972 into fontra:main Jul 8, 2025
5 checks passed
@justvanrossum

Copy link
Copy Markdown
Member

I edited the CHANGELOG, please let me know if it can/should be improved: https://github.com/googlefonts/fontra/blob/main/CHANGELOG.md#2025-07-08

@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

I edited the CHANGELOG, please let me know if it can/should be improved: https://github.com/googlefonts/fontra/blob/main/CHANGELOG.md#2025-07-08

All of this actually just extracted from the basicSetup bundle, which already existed previously

  • added bracket matching
  • added comment toggle (command/control /)
  • added rectangular selection
  • fixed undo/redo
  • and more

@justvanrossum

Copy link
Copy Markdown
Member

So, should I edit down the CHANGELOG entry to the items you list?

@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

So, should I edit down the CHANGELOG entry to the items you list?

Maybe, because I only improved the CodeMirror setup.

@justvanrossum

Copy link
Copy Markdown
Member

I only removed these items:

  • added bracket matching
  • added rectangular selection

I think all other items were indeed added by you. Let me know if you find this correct.

@navv-1

navv-1 commented Jul 8, 2025

Copy link
Copy Markdown
Contributor Author

Okay, undo/redo and toggle comment are part of the basicSetup that was not working previously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants