[PR #98] [MERGED] Support font files as well as font names for the --font flag. #96

New issue

Closed

opened 2026-03-02 16:48:10 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 16:48:10 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/initialcommit-com/git-sim/pull/98
Author: @ehmatthes
Created: 7/16/2023
Status: ✅ Merged
Merged: 7/17/2023
Merged by: @initialcommit-io

Base: dev ← Head: support_font_files

📝 Commits (6)

645c20c Support local font files for --font flag.
6ab291b Handles both font names and font files.
9b2545c Uses contextlib.nullcontext instead of if block.
c6b47a9 Tests use local font file; regenerated reference files.
588477a Fixed message showing full test command.
6533685 Formatted using Black.

📊 Changes

23 files changed (+34 additions, -6 deletions)

View changed files

📝 src/git_sim/__main__.py (+20 -1)
📝 src/git_sim/commands.py (+2 -1)
📝 src/git_sim/settings.py (+1 -0)
➕ tests/e2e_tests/ProggyClean.ttf (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-add.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-branch.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-checkout.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-cherry_pick.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-clean.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-commit.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-log.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-merge.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-mv.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-rebase.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-reset.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-restore.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-revert.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-rm.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-stash.png (+0 -0)
📝 tests/e2e_tests/reference_files/git-sim-status.png (+0 -0)

...and 3 more files

📄 Description

This PR adds support for font files as well as named fonts. Now both of these commands work:

$ git-sim log --font='Courier New'
$ git-sim log --font='/full/path/to/font_file.ttf'

The motivation for this is to address cross-platform testing inconsistencies. There are likely very few end users who would want to run git-sim against a local font file, although as I write that out I start to see use cases. git-sim is quite scriptable, so I could see someone writing a script to generate a series of images or animations, and bundling a font file in order to generate consistent output across different systems.

There are some things to consider before merging this:

This requires the fonttools package, used in __main__.py to pull the name of the font from the font file. I did not add this requirement to pyproject.toml.
I wasn't exactly sure where to put the function get_font_name(). It's currently in __main__.py.
I also wasn't sure exactly where to put the code that processes the font argument. I put it in __main__.py, because that's where most other settings seem to be finalized.
There's a bit of naming difficulty around how the word font is used. It can be a path, it can be a named font, it can be a kind of font. I didn't want to mess with any existing usage, so I made a new setting called font_context. This is set in __main__.py as either a call to m.register_font(), or as a null context. This makes a nice clean change to commands.py, where the call to _handle_animations() is either run in a context that has access to the local font file, or a null context where all behavior is as it was before this PR. There's no need to change any of the individual command files. I was not sure exactly what type to assign to font_context; I chose bool because I don't think register_font() returns anything. It works, but maybe there's a more precise type for it?
get_cmd_parts() in e2e_tests/utils.py needed a bit of reworking, because git-sim is now in the command itself and in the path to the font file. I added a diagnostic print call to show the full test command. You can copy this command, cd to the sample repo, and run this command to see all the output a user would see when running the command. This is really helpful when troubleshooting the test suite itself, and failed tests.
I chose a small, open font to use for the demo of testing with a local font file. It's rather ugly; I kept it in here for the initial PR because it was helpful to use a font that was visibly very different both from Courier New, and from the default font my system uses when the requested font fails. I think it will be helpful to anyone who reviews this PR as well. When this is merged, it might be good to choose an open font that looks more like the output you're used to.
I had to replace all the existing reference files. I ran the tests and they all failed as expected. I went into the sample repo and copied all 18 output files, and pasted them into a folder in a git-sim_support/ directory. I then wrote a small script that renames all of those files to remove the timestamps from the file names. I'll share that script; it makes it quick and easy to update all the reference files when you change git-sim in a way that affects all visual output.
I tested this on Windows. I had to rebuild my environment there. When I installed git-sim, the tests failed because git-dummy did not get placed on the path. I rebuilt the env by installing git-dummy and then git-sim, and things worked again. I'm not sure what caused that.
I generated the reference files on macOS, and on Windows the highest pixel diff was 0.00532. I think we can probably drop the threshold to 0.0075 again, but I'd want to know that things are working on your system, and on Linux before doing that. Side note, I saw that opencv is a dependency of this project, so there might be a cleaner way to compare images at some point. But that would only affect compare_images(), and it wouldn't do anything to address cross-platform issues caused by different fonts.

I doubt this is ready to merge as is, although the remaining work should be a matter of refinement. I think it's worth making the test output consistent if you want the test suite to be useful.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/initialcommit-com/git-sim/pull/98 **Author:** [@ehmatthes](https://github.com/ehmatthes) **Created:** 7/16/2023 **Status:** ✅ Merged **Merged:** 7/17/2023 **Merged by:** [@initialcommit-io](https://github.com/initialcommit-io) **Base:** `dev` ← **Head:** `support_font_files` --- ### 📝 Commits (6) - [`645c20c`](https://github.com/initialcommit-com/git-sim/commit/645c20c55a974c3f5fbe4b50bcb8cb27fdc5b13b) Support local font files for --font flag. - [`6ab291b`](https://github.com/initialcommit-com/git-sim/commit/6ab291bf6a43bc834763000e2c6559a8b5b20dd9) Handles both font names and font files. - [`9b2545c`](https://github.com/initialcommit-com/git-sim/commit/9b2545c315cc11f0b4b991ab50557f7ca670f99a) Uses contextlib.nullcontext instead of if block. - [`c6b47a9`](https://github.com/initialcommit-com/git-sim/commit/c6b47a9aef646a37c96e35353c39ab33911549bc) Tests use local font file; regenerated reference files. - [`588477a`](https://github.com/initialcommit-com/git-sim/commit/588477a845f0fa70d5f81406918269af58bf1d5d) Fixed message showing full test command. - [`6533685`](https://github.com/initialcommit-com/git-sim/commit/65336857dc20050f71f87c82ace7629c55bfe593) Formatted using Black. ### 📊 Changes **23 files changed** (+34 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `src/git_sim/__main__.py` (+20 -1) 📝 `src/git_sim/commands.py` (+2 -1) 📝 `src/git_sim/settings.py` (+1 -0) ➕ `tests/e2e_tests/ProggyClean.ttf` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-add.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-branch.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-checkout.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-cherry_pick.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-clean.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-commit.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-log.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-merge.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-mv.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-rebase.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-reset.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-restore.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-revert.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-rm.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-stash.png` (+0 -0) 📝 `tests/e2e_tests/reference_files/git-sim-status.png` (+0 -0) _...and 3 more files_ </details> ### 📄 Description This PR adds support for font files as well as named fonts. Now both of these commands work: ```sh $ git-sim log --font='Courier New' $ git-sim log --font='/full/path/to/font_file.ttf' ``` The motivation for this is to address cross-platform testing inconsistencies. There are likely very few end users who would want to run git-sim against a local font file, although as I write that out I start to see use cases. git-sim is quite scriptable, so I could see someone writing a script to generate a series of images or animations, and bundling a font file in order to generate consistent output across different systems. There are some things to consider before merging this: - This requires the `fonttools` package, used in \_\_main\_\_.py to pull the name of the font from the font file. I did not add this requirement to pyproject.toml. - I wasn't exactly sure where to put the function `get_font_name()`. It's currently in \_\_main\_\_.py. - I also wasn't sure exactly where to put the code that processes the font argument. I put it in \_\_main\_\_.py, because that's where most other settings seem to be finalized. - There's a bit of naming difficulty around how the word `font` is used. It can be a path, it can be a named font, it can be a kind of font. I didn't want to mess with any existing usage, so I made a new setting called `font_context`. This is set in \_\_main\_\_.py as either a call to `m.register_font()`, or as a null context. This makes a nice clean change to commands.py, where the call to `_handle_animations()` is either run in a context that has access to the local font file, or a null context where all behavior is as it was before this PR. There's no need to change any of the individual command files. I was not sure exactly what type to assign to `font_context`; I chose `bool` because I don't think `register_font()` [returns anything](https://github.com/ManimCommunity/manim/blob/v0.5.0/manim/mobject/svg/text_mobject.py#L1436). It works, but maybe there's a more precise type for it? - `get_cmd_parts()` in e2e_tests/utils.py needed a bit of reworking, because `git-sim` is now in the command itself and in the path to the font file. I added a diagnostic print call to show the full test command. You can copy this command, cd to the sample repo, and run this command to see all the output a user would see when running the command. This is really helpful when troubleshooting the test suite itself, and failed tests. - I chose a [small, open font](https://github.com/bluescan/proggyfonts/blob/master/ProggyOriginal/ProggyClean.ttf) to use for the demo of testing with a local font file. It's rather ugly; I kept it in here for the initial PR because it was helpful to use a font that was visibly very different both from Courier New, and from the default font my system uses when the requested font fails. I think it will be helpful to anyone who reviews this PR as well. When this is merged, it might be good to choose an open font that looks more like the output you're used to. - I had to replace all the existing reference files. I ran the tests and they all failed as expected. I went into the sample repo and copied all 18 output files, and pasted them into a folder in a git-sim_support/ directory. I then wrote a small script that renames all of those files to remove the timestamps from the file names. I'll share that script; it makes it quick and easy to update all the reference files when you change git-sim in a way that affects all visual output. - I tested this on Windows. I had to rebuild my environment there. When I installed git-sim, the tests failed because git-dummy did not get placed on the path. I rebuilt the env by installing git-dummy and then git-sim, and things worked again. I'm not sure what caused that. - I generated the reference files on macOS, and on Windows the highest pixel diff was 0.00532. I think we can probably drop the threshold to 0.0075 again, but I'd want to know that things are working on your system, and on Linux before doing that. Side note, I saw that opencv is a dependency of this project, so there might be a cleaner way to compare images at some point. But that would only affect `compare_images()`, and it wouldn't do anything to address cross-platform issues caused by different fonts. I doubt this is ready to merge as is, although the remaining work should be a matter of refinement. I think it's worth making the test output consistent if you want the test suite to be useful. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>