{"thread":{"id":"62049","subject":"[filter-repo PATCH]: add --callbacks option to load many callbacks from one file","startedAt":"2024-09-03T19:43:07Z","lastAt":"2024-09-04T15:14:22Z","messageCount":3,"participants":["Zack Weinberg","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"502061","messageId":"20240903194244.16709-1-zack@owlfolio.org","threadId":"62049","inReplyTo":null,"subject":"[filter-repo PATCH]: add --callbacks option to load many callbacks from one file","fromName":"Zack Weinberg","fromEmail":"zack@owlfolio.org","sentAt":"2024-09-03T19:42:44Z","receivedAt":"2024-09-03T19:43:07Z","isPatch":true,"sender":{"key":"zack@owlfolio.org","avatar":null},"body":"If you are trying to do something complicated with filter-repo,\nyou might need to share state among several callbacks, which is\ncurrently impossible (short of poking values into someone else’s\nnamespace) because each callback defined on the command line gets\nits own globals dictionary.\n\nOr, if you are trying to do something simple but long-winded, such as\nreplacing the entire contents of a file, you might want to define long\nmulti-line (byte) strings as global variables, to avoid having to deal\nwith the undocumented number of spaces inserted at the beginning of\neach line by the callback parser.\n\nTo facilitate these kinds of uses, add a new command line option\n`--callbacks`.  The argument to this option is a file, which should\ndefine callback functions, using the same naming convention as is\ndescribed for individual command line callbacks, e.g.\n\n    def name_callback(name):\n        ...\n\nto set the name callback.  Any Python callable is acceptable, e.g.\n\n    class Callbacks:\n        def name_callback(self, name):\n            ...\n\n    callbacks = Callbacks()\n    name_callback = callbacks.name_callback\n\nwill also work.  People who know about the undocumented second argument\nto some callbacks may define callbacks that take two arguments.\n\nThe callbacks file is loaded as an ordinary Python module; it does _not_\nget any automatic globals, unlike individual command line callbacks.\nHowever, `import git_format_repo` will work inside the callbacks file,\neven if git_format_repo.py has not been made available in general.\n\nTests are added which lightly exercise the new feature, and I have\nalso used it myself for a real repo rewrite (of the “simple but\nlong-winded” variety).\n\n----\n\nAlso document (briefly) the existing feature of supplying a file\nname rather than an inline function body to --foo-callback options,\nand the availability of an unspecified set of globals to individual\ncallbacks (with instruction to see the source code for details).\n\nThis patch introduces uses of the Python standard library modules\nerrno, importlib, and inspect.  All functionality used from these\nmodules was available in 3.6 or earlier.\n\nThis patch introduces several new translatable strings and changes\none existing translatable string.\n\n----\n\nSigned-off-by: Zack Weinberg <zack@owlfolio.org>\n---\n Documentation/git-filter-repo.txt |  39 ++++++-\n git-filter-repo                   | 164 +++++++++++++++++++++++-------\n t/t9391-filter-repo-lib-usage.sh  |   2 +-\n t/t9392-python-callback.sh        |  57 ++++++++++-\n 4 files changed, 219 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/git-filter-repo.txt b/Documentation/git-filter-repo.txt\nindex 85cd5b9..09128e4 100644\n--- a/Documentation/git-filter-repo.txt\n+++ b/Documentation/git-filter-repo.txt\n@@ -1084,7 +1084,7 @@ For flexibility, filter-repo allows you to specify functions on the\n command line to further filter all changes.  Please note that there\n are some API compatibility caveats associated with these callbacks\n that you should be aware of before using them; see the \"API BACKWARD\n-COMPATIBILITY CAVEAT\" comment near the top of git-filter-repo source\n+COMPATIBILITY CAVEAT\" comment near the top of the git-filter-repo source\n code.\n \n All callback functions are of the same general format.  For a command line\n@@ -1102,10 +1102,39 @@ def foo_callback(foo):\n --------------------------------------------------\n \n Thus, you just need to make sure your _BODY_ modifies and returns\n-_foo_ appropriately.  One important thing to note for all callbacks is\n-that filter-repo uses bytestrings (see\n-https://docs.python.org/3/library/stdtypes.html#bytes) everywhere\n-instead of strings.\n+_foo_ appropriately.  Alternatively, _BODY_ can be the name of a file,\n+in which case the function body is read from that file.\n+\n+Callback functions defined this way have access to all the standard\n+library modules imported by git-filter-repo itself, plus its public\n+library API; see the `public_globals` variable, near the top of\n+git-filter-repo, for the exact list.\n+\n+Callback functions can also be defined in a group:\n+\n+--------------------------------------------------\n+--callbacks FILE\n+--------------------------------------------------\n+\n+will load FILE as a Python module.  FILE should define functions\n+(actually, any Python callable will do) for each of the callbacks you\n+wish to use, with names like `foo_callback`, where `foo` corresponds\n+to a `--foo-callback` command line option.  This can be useful if you\n+need to share state among your callbacks, or do some preparation in\n+advance of the first call.\n+\n+Callback functions defined this way are _not_ given access to any\n+modules or globals that FILE doesn’t import for itself.  However,\n+`import git_filter_repo` will work inside FILE, whether or not\n+`git_filter_repo.py` has been installed (see `INSTALL.md` in the\n+source tree for further explanation).\n+\n+There can be only one callback function of a particular type, however\n+you define it.\n+\n+When writing callbacks, keep in mind that git-filter-repo uses\n+bytestrings (see https://docs.python.org/3/library/stdtypes.html#bytes)\n+everywhere, instead of strings.\n \n There are four callbacks that allow you to operate directly on raw\n objects that contain data that's easy to write in\ndiff --git a/git-filter-repo b/git-filter-repo\nindex 9cce52a..7334c2d 100755\n--- a/git-filter-repo\n+++ b/git-filter-repo\n@@ -32,8 +32,10 @@ operations; however:\n \n import argparse\n import collections\n+import errno\n import fnmatch\n import gettext\n+import inspect\n import io\n import os\n import platform\n@@ -53,10 +55,10 @@ __all__ = [\"Blob\", \"Reset\", \"FileChange\", \"Commit\", \"Tag\", \"Progress\",\n \n # The globals to make visible to callbacks. They will see all our imports for\n # free, as well as our public API.\n-public_globals = [\"__builtins__\", \"argparse\", \"collections\", \"fnmatch\",\n-                  \"gettext\", \"io\", \"os\", \"platform\", \"re\", \"shutil\",\n-                  \"subprocess\", \"sys\", \"time\", \"textwrap\", \"tzinfo\",\n-                  \"timedelta\", \"datetime\"] + __all__\n+public_globals = [\"__builtins__\", \"argparse\", \"collections\", \"errno\",\n+                  \"fnmatch\", \"gettext\", \"inspect\", \"io\", \"os\", \"platform\",\n+                  \"re\", \"shutil\", \"subprocess\", \"sys\", \"time\", \"textwrap\",\n+                  \"tzinfo\", \"timedelta\", \"datetime\"] + __all__\n \n deleted_hash = b'0'*40\n write_marks = True\n@@ -1719,6 +1721,9 @@ class FilteringOptions(object):\n       def foo_callback(foo):\n         BODY\n \n+    Alternatively, BODY can be a filename; then the contents of that file\n+    will be used as the BODY in the callback function.\n+\n     Thus, to replace 'Jon' with 'John' in author/committer/tagger names:\n       git filter-repo --name-callback 'return name.replace(b\"Jon\", b\"John\")'\n \n@@ -1728,8 +1733,14 @@ class FilteringOptions(object):\n     To remove all .DS_Store files:\n       git filter-repo --filename-callback 'return None if os.path.basename(filename) == b\".DS_Store\" else filename'\n \n-    Note that if BODY resolves to a filename, then the contents of that file\n-    will be used as the BODY in the callback function.\n+    You can also use the --callbacks option to define several callback\n+    functions at once:\n+\n+      git filter-repo --callbacks my_callbacks.py\n+\n+    my_callbacks.py will be parsed as a Python module.  It should\n+    define functions with names like 'foo_callback', corresponding to\n+    the various --foo-callback options.\n \n     For more detailed examples and explanations AND caveats, see\n       https://htmlpreview.github.io/?https://github.com/newren/git-filter-repo/blob/docs/html/git-filter-repo.html#CALLBACKS\n@@ -1983,6 +1994,10 @@ EXAMPLES\n         help=_(\"Python code body for processing reset objects; see \"\n                \"CALLBACKS section below.\"))\n \n+    callback.add_argument('--callbacks', metavar=\"FILE\",\n+        help=_(\"File defining callback functions, to be loaded as a \"\n+               \"Python module; see CALLBACKS section below.\"))\n+\n     desc = _(\n       \"Specifying alternate source or target locations implies --partial,\\n\"\n       \"except that the normal default for --replace-refs is used.  However,\\n\"\n@@ -2259,6 +2274,104 @@ EXAMPLES\n       args.refs = ['--all']\n     return args\n \n+class Callbacks(object):\n+  ''' A set of callback functions; handles fabricating such functions\n+      from the command line arguments. '''\n+\n+  TYPES = [\n+    'blob', 'commit', 'tag', 'reset', 'done',\n+    'filename', 'message', 'name', 'email', 'refname'\n+  ]\n+\n+  @staticmethod\n+  def load_callbacks_module(fname):\n+    ''' Load FNAME as a module object; returns that module object. '''\n+    # Make \"import git_filter_repo\" work inside the file we're loading,\n+    # whether or not git_filter_repo.py has been installed.\n+    if 'git_filter_repo' not in sys.modules:\n+      sys.modules['git_filter_repo'] = sys.modules[__name__]\n+\n+    # this recipe almost verbatim from\n+    # https://docs.python.org/3.9/library/importlib.html#importing-a-source-file-directly\n+    # (documented to work in 3.5 and later)\n+    from importlib import util\n+    spec = util.spec_from_file_location('git_filter_repo.callbacks', fname)\n+    module = util.module_from_spec(spec)\n+    sys.modules['git_filter_repo.callbacks'] = module\n+    spec.loader.exec_module(module)\n+\n+    return module\n+\n+  @staticmethod\n+  def parse_single_callback(cb_type, cb_name, fname_or_body):\n+    try:\n+      with open(fname_or_body, \"rt\") as fp:\n+        body = fp.read()\n+    except OSError as e:\n+      # There is no dedicated OSError subclass for \"name too long\".\n+      if e.errno in (errno.ENOENT, errno.ENAMETOOLONG):\n+        body = fname_or_body\n+      else:\n+        raise\n+\n+    if 'return ' not in body and cb_type not in ('blob', 'commit',\n+                                                 'tag', 'reset'):\n+      raise SystemExit(\n+        _(\"Error: --%s-callback should have a return statement\") % cb_type\n+      )\n+\n+    def_stmt = (\n+      'def {name}({argname}, _do_not_use_this_var = None):\\n'\n+      .format(name=cb_name, argname=cb_type)\n+      + '  '\n+      + '\\n  '.join(body.splitlines())\n+    )\n+    callback_globals = {g: globals()[g] for g in public_globals}\n+    callback_locals = {}\n+    exec(def_stmt, callback_globals, callback_locals)\n+    return callback_locals[cb_name]\n+\n+  @staticmethod\n+  def make_callback(cb_type, cb_name, mod, fname_or_body):\n+    cb = None\n+\n+    if mod is not None:\n+      cb = getattr(mod, cb_name, None)\n+      if cb is not None:\n+        if not callable(cb):\n+          raise SystemExit(_(\"Error: %s is not callable\") % cb_name)\n+        # the second argument to blob, commit, tag, and reset filters is\n+        # not documented as part of the command line callbacks API; allow\n+        # people using --callbacks to define callbacks with only one argument\n+        sig = inspect.signature(cb)\n+        if len(sig.parameters) == 1:\n+          real_cb = cb\n+          def wrapper(obj, _unused = None):\n+            return real_cb(obj)\n+          cb = wrapper\n+\n+    if fname_or_body is not None:\n+      if cb is not None:\n+        raise SystemExit(_(\n+          \"Error: Cannot define %s_callback in --callbacks module and also\"\n+          \" use --%s-callback\"\n+        ) % (cb_type, cb_type))\n+      cb = Callbacks.parse_single_callback(cb_type, cb_name, fname_or_body)\n+\n+    return cb\n+\n+  def __init__(self, args):\n+    if args.callbacks is None:\n+      mod = None\n+    else:\n+      mod = Callbacks.load_callbacks_module(args.callbacks)\n+\n+    for cb_type in Callbacks.TYPES:\n+      cb_name = cb_type + '_callback'\n+      setattr(self, cb_type, Callbacks.make_callback(\n+        cb_type, cb_name, mod, getattr(args, cb_name, None)\n+      ))\n+\n class RepoAnalyze(object):\n \n   # First, several helper functions for analyze_commit()\n@@ -2877,37 +2990,16 @@ class RepoFilter(object):\n     self._full_hash_re = re.compile(br'(\\b[0-9a-f]{40}\\b)')\n \n   def _handle_arg_callbacks(self):\n-    def make_callback(argname, str):\n-      callback_globals = {g: globals()[g] for g in public_globals}\n-      callback_locals = {}\n-      exec('def callback({}, _do_not_use_this_var = None):\\n'.format(argname)+\n-           '  '+'\\n  '.join(str.splitlines()), callback_globals, callback_locals)\n-      return callback_locals['callback']\n-    def handle(type):\n-      callback_field = '_{}_callback'.format(type)\n-      code_string = getattr(self._args, type+'_callback')\n-      if code_string:\n-        if os.path.exists(code_string):\n-          with open(code_string, 'r', encoding='utf-8') as f:\n-            code_string = f.read()\n-        if getattr(self, callback_field):\n+    arg_callbacks = Callbacks(self._args)\n+    for cb_type in Callbacks.TYPES:\n+      callback_field = '_{}_callback'.format(cb_type)\n+      arg_cb = getattr(arg_callbacks, cb_type)\n+      if arg_cb is not None:\n+        if getattr(self, callback_field) is not None:\n           raise SystemExit(_(\"Error: Cannot pass a %s_callback to RepoFilter \"\n-                             \"AND pass --%s-callback\"\n-                           % (type, type)))\n-        if 'return ' not in code_string and \\\n-           type not in ('blob', 'commit', 'tag', 'reset'):\n-          raise SystemExit(_(\"Error: --%s-callback should have a return statement\")\n-                           % type)\n-        setattr(self, callback_field, make_callback(type, code_string))\n-    handle('filename')\n-    handle('message')\n-    handle('name')\n-    handle('email')\n-    handle('refname')\n-    handle('blob')\n-    handle('commit')\n-    handle('tag')\n-    handle('reset')\n+                             \"AND define it on the command line\"\n+                             % cb_type))\n+        setattr(self, callback_field, arg_cb)\n \n   def _run_sanity_checks(self):\n     self._sanity_checks_handled = True\ndiff --git a/t/t9391-filter-repo-lib-usage.sh b/t/t9391-filter-repo-lib-usage.sh\nindex 3a86961..daf1d57 100755\n--- a/t/t9391-filter-repo-lib-usage.sh\n+++ b/t/t9391-filter-repo-lib-usage.sh\n@@ -157,7 +157,7 @@ test_expect_success 'erroneous.py' '\n \t\tcd erroneous &&\n \t\ttest_must_fail $TEST_DIRECTORY/t9391/erroneous.py 2>../err &&\n \n-\t\ttest_i18ngrep \"Error: Cannot pass a tag_callback to RepoFilter AND pass --tag-callback\" ../err\n+\t\ttest_i18ngrep \"Error: Cannot pass a tag_callback to RepoFilter AND define it\" ../err\n \t)\n '\n \ndiff --git a/t/t9392-python-callback.sh b/t/t9392-python-callback.sh\nindex cb36292..cafb9cf 100755\n--- a/t/t9392-python-callback.sh\n+++ b/t/t9392-python-callback.sh\n@@ -181,7 +181,7 @@ test_expect_success 'callback has return statement sanity check' '\n \t)\n '\n \n-test_expect_success 'Callback read from a file' '\n+test_expect_success 'callback read from a file' '\n \tsetup name-callback-from-file &&\n \t(\n \t\tcd name-callback-from-file &&\n@@ -192,5 +192,60 @@ test_expect_success 'Callback read from a file' '\n \t)\n '\n \n+test_expect_success 'callback defined in a module' '\n+\tsetup name-callback-from-module &&\n+\t(\n+\t\tcd name-callback-from-module &&\n+\t\tcat >> ../callbacks.py <<\\EOF &&\n+def name_callback(name):\n+    return name.replace(b\"N.\", b\"And\")\n+EOF\n+\t\tgit filter-repo --callbacks ../callbacks.py &&\n+\t\tgit log --format=%an >log-person-names &&\n+\t\tgrep Copy.And.Paste log-person-names\n+\t)\n+'\n+\n+test_expect_success 'friendly error when module callbacks are not callable' '\n+\tsetup bad-callback-friendly-error &&\n+\t(\n+\t\tcd bad-callback-friendly-error &&\n+\t\tcat >> ../bad-callbacks.py <<\\EOF &&\n+name_callback = \"not a callable\"\n+EOF\n+\t\ttest_must_fail git filter-repo --callbacks ../bad-callbacks.py 2>../err &&\n+\t\ttest_i18ngrep \"Error: name_callback is not callable\" ../err &&\n+\t\trm ../err\n+\t)\n+'\n+\n+test_expect_success 'module/cmdline callback collision' '\n+\tsetup mod-cmdline-callback-collision &&\n+\t(\n+\t\tcd mod-cmdline-callback-collision &&\n+\t\tcat >> ../mccoll-callbacks.py <<\\EOF &&\n+def name_callback(name):\n+    return name.replace(b\"N.\", b\"And\")\n+EOF\n+\t\tcat >> ../mccoll-name-cb <<\\EOF &&\n+return name.replace(b\"N.\", b\"And\")\n+EOF\n+\t\ttest_must_fail git filter-repo --callbacks ../mccoll-callbacks.py --name-callback ../mccoll-name-cb 2>../err &&\n+\t\ttest_i18ngrep \"Error: Cannot define name_callback in --callbacks module and also use --name-callback\" ../err &&\n+\t\trm ../err\n+\t)\n+'\n+\n+test_expect_success 'module callbacks can import git_filter_repo' '\n+\tsetup mod-callbacks-can-import &&\n+\t(\n+\t\tcd mod-callbacks-can-import &&\n+\t\tcat >> ../import-test-callbacks.py <<\\EOF &&\n+import git_filter_repo\n+EOF\n+\t\tgit filter-repo --callbacks ../import-test-callbacks.py\n+\t)\n+'\n+\n \n test_done\n-- \n2.44.2\n\n"},{"id":"502075","messageId":"CABPp-BFqbiS8xsbLouNB41QTc5p0hEOy-EoV0Sjnp=xJEShkTw@mail.gmail.com","threadId":"62049","inReplyTo":"20240903194244.16709-1-zack@owlfolio.org","subject":"Re: [filter-repo PATCH]: add --callbacks option to load many callbacks from one file","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-09-03T22:11:16Z","receivedAt":"2024-09-03T22:11:29Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hey,\n\nThanks for sending this in.\n\nOn Tue, Sep 3, 2024 at 12:43 PM Zack Weinberg <zack@owlfolio.org> wrote:\n>\n> If you are trying to do something complicated with filter-repo,\n> you might need to share state among several callbacks, which is\n> currently impossible (short of poking values into someone else’s\n> namespace) because each callback defined on the command line gets\n> its own globals dictionary.\n\nSharing state is not impossible; it's done in multiple examples in\nfilter-repo.  If you want to do something complicated with\nfilter-repo, you can just import it as a library.\ncontrib/filter-repo-demos includes several more complicated examples,\nincluding complete replacements for git-filter-branch and\nbfg-repo-cleaner.  Let's look at how some of them hook up callbacks:\n\n$ git grep 'callback=.*\\.' contrib/\ncontrib/filter-repo-demos/bfg-ish:    self.filter =\nfr.RepoFilter(fr_args, commit_callback=self.commit_update)\ncontrib/filter-repo-demos/clean-ignore:  filter = fr.RepoFilter(args,\ncommit_callback=checker.skip_ignores)\ncontrib/filter-repo-demos/filter-lamely:\n commit_callback=self.fixup_commit,\ncontrib/filter-repo-demos/filter-lamely:\n refname_callback=self.tag_rename,\ncontrib/filter-repo-demos/filter-lamely:\n tag_callback=self.deref_tags)\n\nThe fact that these callbacks point towards an instance method\nfunction makes it clear that these functions will all have access to\nthe relevant instance ('self' or 'checker' in the examples above),\neven when they are different callbacks.  They also have wider access\nto whatever globals you might want to define in that file as well;\nit's only the simple one-off defined-at-the-command-line callbacks\nthat were geared towards the simplistic cases that have a separation.\n\n> Or, if you are trying to do something simple but long-winded, such as\n> replacing the entire contents of a file, you might want to define long\n> multi-line (byte) strings as global variables, to avoid having to deal\n> with the undocumented number of spaces inserted at the beginning of\n> each line by the callback parser.\n\nYeah, I can see how the added spaces would be slightly annoying for\nthe case of multi-line strings (though simple callbacks like\n`--name-callback 'return name.replace(b\"Wiliam\", b\"William\")'` require\nthat some kind of leading whitespace be added, and the command line\n--*-callback options are targetted towards the simpler usecases, after\nall).  However, even in that case you can just use textwrap.dedent.\nFor example:\n\ngit filter-repo --blob-callback '\n  import textwrap\n  blob.data = bytes(textwrap.dedent(\"\"\"\\\n    This is the new\n    file that I am\n    replacing every blob\n    with.  It is great.\n    \"\"\"), \"utf-8\")\n'\n\nAnd now every file in your repository is replaced by one with the\nfollowing contents:\n\"\"\"\nThis is the new\nfile that I am\nreplacing every blob\nwith.  It is great.\n\"\"\"\nNotice the lack of leading spaces.\n\n> To facilitate these kinds of uses, add a new command line option\n> `--callbacks`.  The argument to this option is a file, which should\n> define callback functions, using the same naming convention as is\n> described for individual command line callbacks, e.g.\n>\n>     def name_callback(name):\n>         ...\n>\n> to set the name callback.  Any Python callable is acceptable, e.g.\n>\n>     class Callbacks:\n>         def name_callback(self, name):\n>             ...\n>\n>     callbacks = Callbacks()\n>     name_callback = callbacks.name_callback\n>\n> will also work.  People who know about the undocumented second argument\n> to some callbacks may define callbacks that take two arguments.\n>\n> The callbacks file is loaded as an ordinary Python module; it does _not_\n> get any automatic globals, unlike individual command line callbacks.\n> However, `import git_format_repo` will work inside the callbacks file,\n> even if git_format_repo.py has not been made available in general.\n\nWhat is this git_format_repo thing you speak of?\n\nAnyway, separate from that and given the above comments I made about\nimporting git_filter_repo and textwrap.dedent, I'm a little unsure why\nthis new callbacks command line flag helps.  The usage of\ngit_filter_repo.py as a library exists for the general case already.\nSimple command line flags like `--path` or `--replace-text` exist to\nhandle the simplest cases.  All the *-callback command line flags\nexist to provide a middle ground where the user needs something a bit\nmore generic and programmatic, but is still targeting a certain piece\nof data from the fast-export stream and doesn't want the little extra\nverbosity from placing things in a separate file and importing\ngit_filter_repo and the few lines of glue necessary to hook it up.\n\nIn that scheme, this new --callbacks option doesn't seem to \"fit\" to\nme.  It makes the middle ground more generic, but not as generic as\nthe ability to just import git_filter_repo and do whatever you want.\nThere's no specific targeting it provides, which makes it feel to me\nas there's no specific reason for it to exist separate from the more\ngeneric usecase.\n\n> Tests are added which lightly exercise the new feature, and I have\n> also used it myself for a real repo rewrite (of the “simple but\n> long-winded” variety).\n>\n> ----\n>\n> Also document (briefly) the existing feature of supplying a file\n> name rather than an inline function body to --foo-callback options,\n> and the availability of an unspecified set of globals to individual\n> callbacks (with instruction to see the source code for details).\n\nThanks for being helpful.  Do note, though, that different logical\nchanges should be in separate patches.\n\n> This patch introduces uses of the Python standard library modules\n> errno, importlib, and inspect.  All functionality used from these\n> modules was available in 3.6 or earlier.\n\nSince I recently bumped the minimum required python to 3.6, this is\nall good.  Thanks for calling it out.\n\n> This patch introduces several new translatable strings and changes\n> one existing translatable string.\n\nAgain, thanks for taking the time to call this out.\n\n> ---\n>  Documentation/git-filter-repo.txt |  39 ++++++-\n>  git-filter-repo                   | 164 +++++++++++++++++++++++-------\n>  t/t9391-filter-repo-lib-usage.sh  |   2 +-\n>  t/t9392-python-callback.sh        |  57 ++++++++++-\n>  4 files changed, 219 insertions(+), 43 deletions(-)\n\nI skimmed over the patch briefly, and it seems to generally be\nreasonable, but I didn't look real close since I'm not sure if the\nfeature \"fits\".  Does the knowledge about textwrap.dedent or the\nability to import git_filter_repo help you out?\n\n\n\nHope that helps,\nElijah\n"},{"id":"502146","messageId":"7cfe4a14-c319-4ccd-9c1d-099139a88913@app.fastmail.com","threadId":"62049","inReplyTo":"CABPp-BFqbiS8xsbLouNB41QTc5p0hEOy-EoV0Sjnp=xJEShkTw@mail.gmail.com","subject":"Re: [filter-repo PATCH]: add --callbacks option to load many callbacks from one file","fromName":"Zack Weinberg","fromEmail":"zack@owlfolio.org","sentAt":"2024-09-04T15:13:43Z","receivedAt":"2024-09-04T15:14:22Z","isPatch":true,"sender":{"key":"zack@owlfolio.org","avatar":null},"body":"On Tue, Sep 3, 2024, at 6:11 PM, Elijah Newren wrote:\n> On Tue, Sep 3, 2024 at 12:43 PM Zack Weinberg\n> <zack@owlfolio.org> wrote:\n>> If you are trying to do something complicated with filter-repo,\n...\n> If you want to do something complicated with filter-repo, you can just\n> import it as a library.\n\nI should have said up front that I see this proposed feature as\nfilling a hole in the power/convenience tradeoff space, between the\nindividual --foo-callback switches and using filter-repo as a library.\nIt facilitates doing things that are a little bit too hard if you want\nto use individual switches, but not so hard that it feels worthwhile to\nstart reading the \"using filter-repo as a library\" examples and braving\nthe internal API stability warnings.\n\nIn particular, it gives you the programming environment that you're\naccustomed to if you're an experienced Python coder -- \"this file will\nbe parsed as a Python module\" is much more \"normal\" to such people than\n\"this file will be parsed as the body of a function\" -- but stays within\nthe lines of the official stable callback API.\n\nIt also works without setting up the _capability_ to use filter-repo as\na library, so that saves at least one preparatory step.\n\n>> Or, if you are trying to do something simple but long-winded, such as\n>> replacing the entire contents of a file, you might want to define\n>> long multi-line (byte) strings as global variables, to avoid having\n>> to deal with the undocumented number of spaces inserted at the\n>> beginning of each line by the callback parser.\n>\n> Yeah, I can see how the added spaces would be slightly annoying for\n> the case of multi-line strings (though simple callbacks like `--name-\n> callback 'return name.replace(b\"Wiliam\", b\"William\")'` require that\n> some kind of leading whitespace be added, and the command line --*-\n> callback options are targetted towards the simpler usecases, after\n> all).  However, even in that case you can just use textwrap.dedent.\n\ntextwrap.dedent is great, but you have to know that it exists.  If\nyou're staring at the filter-repo manpage and trying to figure out how\nto replace a multi-line string and you haven't memorized the entire set\nof capabilities of the Python stdlib, \"oh hey I can use --callbacks and\nthen I can put big strings in global variables\" is an easier cognitive\nstretch than _either_ \"oh hey I can use textwrap.dedent\" or \"I guess I\ngotta figure out how to use this unstable library API that's only\nvaguely touched on in the manpage\".\n\nI should also mention that I started developing this feature before I\nknew about the possibility of passing a file name to --foo-callback.\n(The present state of play is that this is documented in --help but not\nin the manpage, and I was only looking at the manpage until I started\ncoding.) The thought of trying to get an entire file's worth of English\ntext through both Python and shell string quotation was too daunting to\ncontemplate.\n\n>> The callbacks file is loaded as an ordinary Python module; it does\n>> _not_ get any automatic globals, unlike individual command line\n>> callbacks. However, `import git_format_repo` will work inside the\n>> callbacks file, even if git_format_repo.py has not been made\n>> available in general.\n>\n> What is this git_format_repo thing you speak of?\n\nDoh! I meant git_filter_repo.\n\n> Anyway, separate from that and given the above comments I made about\n> importing git_filter_repo and textwrap.dedent, I'm a little unsure why\n> this new callbacks command line flag helps.\n\nI hope I have sufficiently explained the rationale above?\n\n>> Also document (briefly) the existing feature of supplying a file name\n>> rather than an inline function body to --foo-callback options, and\n>> the availability of an unspecified set of globals to individual\n>> callbacks (with instruction to see the source code for details).\n>\n> Thanks for being helpful.  Do note, though, that different logical\n> changes should be in separate patches.\n\nIf the overall change is approved, I can split it up into a series. I do\nthink that the --foo-callback <filename> feature should be documented in\nthe manpage regardless of whether --callbacks is added.\n\nzw\n"}]}