Skip to content

Conversation

sandr01d
Copy link
Collaborator

@sandr01d sandr01d commented Sep 6, 2025

Check list

  • I have performed a self-review of my code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation

Description

While playing around with #464, I noticed that the preview of forgit add and forgit clean do not work with files that have backslashes in their names. The preview just shows something like

error: Could not access 'file\.txt'

This is due to the same issue we tried to address with #388 (git escapes special characters in its output, but does not expect them to be escaped when used for input). Since #388 we use the -z option with most git commands that have this "issue" to prevent them from escaping their output. In forgit add and forgit clean we use the core.quotePath false config option instead. In forgit add because we make use of the colors of the output of git status and in forgit clean because git clean does not have a -z option.
I noticed that (in contrast to my previous belief), -z does not remove the colors from git status, so it can be used in forgit add. However, I could not find a way to get things working with our current implementation, but I found a simpler approach to what we're trying to achieve that actually does work. We try to filter out added files using grep by matching lines with changed, unmerged and untracked files. Instead we can just do an inverse match for added files.
For forgit clean I've modified the command to use _forgit_list_files instead of git clean -n. This comes with a minor breaking change: flag arguments provided to forgit clean are now passed to git ls-files instead of git clean -n. However, git ls-files is way more flexible than git clean, so all use cases covered by git clean (and more) should still be possible. This will even allow us to introduce an environment variable in the future to modify which files are included when cleaning (I'll explain my use case in another PR, this text is already way too long 😉).

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation change

Test environment

  • Shell
    • bash
    • zsh
    • fish
  • OS
    • Linux
    • Mac OS X
    • Windows
    • Others:

This is a breaking change: flag arguments provided to forgit clean are
now passed to git ls-files instead of git clean -n. This change is
necessary because git clean does not have a -z option that would prevent
it from escaping file names containing backslashes. However, git ls-files
is way more flexible than git clean, so all use cases covered by git
clean (and more) should still be possible.
@carlfriedrich
Copy link
Collaborator

@sandr01d Good catch! Before diving into the code changes in more detail, I would like to wait until #464 is merged and then approach this in a TDD-way, e.g. implement a failing testcase first and then show that the fix makes the test pass. WDYT?

@sandr01d
Copy link
Collaborator Author

sandr01d commented Sep 6, 2025

I would like to wait until #464 is merged and then approach this in a TDD-way,

Yes, makes sense

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants