Nickolai Belakovski <nbelakov...@gmail.com> writes: > Yes, the parser used the atom argument in an earlier version of this > patch, but we since moved the map out of the atom since it only needs > to exist once globally. Even though we have a caching mechanism for > atoms it still seemed like a logical move to explicitly keep one > instance of the map globally.
I think that is a mistaken move from an earlier version to this one. The worktree-related stuff only becomes necessary and belongs to the %(worktreepath) atom, so unless there is a compelling reason not to, we should hang it there, instead of introducing a global. When you have --format='%(worktreepath) %(worktreepath)', you'd have only one shared instance of it in used_atom[] to ensure that we have a singleton instance. The object_info support in the file, which is relatively new, may be what you borrowed the wrong idea of preferring globals from; I think it should be taken as an anti-pattern.
