-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
Idea: explicitly look for merge conflict markers #23
Comments
why not just use that? |
Possible, but introduces pre-commit. Then you need to go and set up caches for it because the setup takes a minute or more... |
Read every UFO text file and test for these markers? Or are they happening at a significant frequency in specific file area so that we can narrow the list? |
pre-commit is primarily for local development. you install it once then it does its thing. |
more interesting if you turned ufolint into a pre-commit hook (ok now I shut up 😁) |
no that's an interesting idea Cosimo. Details? |
@madig what are you seeing in the error message when this happens? |
a pre-commit hook can be any script that exits with nonzero on failure; it can also modify files in the working directory (e.g. see black) |
Problem is getting contributors to set this up... :) |
i'm sure Nikolaus is already concocting something 😉 |
This is a sticking point. I'm using a build tool wrapper internally at work but not everyone uses it, and people struggle with Git as is. I set up ufolint on the CI for two projects today, but the designers were very concerned about false positives preventing them from building the fonts on the CI (font developers are under constant time pressure), so I allow the job to fail and proceed to building anyway. Setting this up as a pre-commit hook would make little sense like that. We'll see.
|
|
|
https://github.com/unified-font-object/ufoNormalizer FFR What does it do? The docs are... sparse :)
Have any of your designers tried the git GUI applications? Git Kraken is fantastic. I use it all the time. It is rare that I need to drop to the command line for routine work. It would give them visual representation of their branches vs. other remote branches and a set of buttons to click for branching/pulls/pushes. And notifies, adds support for simple conflict resolution when there is a merge conflict in a way that is less daunting than command line git. I'm sure that there are free tools that do the same, but I haven't tried so can't recommend. |
Some do, me and a few others like to use Fork. One of the major problems with e.g. Glyphs in the workflow is that both Glyphs and glyphsLib are imperfect and designers regularly get swamped with humongous amounts of git diffs for innocuous changes depending on glyphsLib version, Glyphs version and moonphase. Then there's stuff generated by tools, e.g. things that work on hinting. What if you did many changes and did not commit in atomic steps? Are you pro enough yet to understand every change done to a UFO? Can you build and diff the textual changes to a .glif in your head? Many throw their hands up and just commit everything and I can understand that. Oh, hello there merge conflict! Font development is and remains a major PITA because the (graphical) tooling just isn't there yet. And that's before you start dealing with application quirks (hello Office for Mac!)
Many things, don't know all of them, but formatting XML in one way and sorting keys are among them iirc. We use it for round-tripping to Glyphs and back due to reordering of keys, whitespace, ... |
let's unilaterally and opportunistically declare that whatever is the output of fontTools.ufoLib is the "normalized" representation of a UFO data, and be done with ufonormalizers. Just sayin' |
What to do about editors that reorder keys? Does ufoLib order them back? |
what order are you talking about? the property list dictionary are always sorted alphabetically. |
and plist arrays area inherently ordered, if authoring tools change their order, that's a bug |
tabs |
Maybe a pre-commit hook for that? |
Worth a try I guess? A script that opens all UFOs, loads everything/marks everything dirty without actually doing anything and writes it back. Can even run on the CI first. |
No one will ever be able to do this. But that is where the tooling like the browser visual diff tool (can't remember name) should come in to play. There needs to be a visual diff level of automation because sometimes changes in the source text do not lead to visual differences, or only do at certain sizes. I have yet to find a tool that allows for simple visual diffs that include size specific rendering with TT instruction sets considered. That is a tool that I would love to see. I don't know how to make it... |
Yes, and it's why there's still years of effort ahead of us :) |
Part of what makes type source special :) It is not simple to interpret for humans, even in text format. And then you build with it and pile on post compilation modifications as icing on the cake. |
Happens every so often. They are caught anyway because they break the XML, but the error messages may be cryptic.
Maybe adapt https://github.com/pre-commit/pre-commit-hooks/blob/master/pre_commit_hooks/check_merge_conflict.py to ufolint.
The text was updated successfully, but these errors were encountered: