D6167: fix: allow fixer tools to return metadata in addition to the file content

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Mon Apr 22 18:10:43 EDT 2019


martinvonz added inline comments.

INLINE COMMENTS

> fix.py:559-570
> +            newerdata = stdout
> +            if fixer.shouldoutputmetadata():
> +                try:
> +                    metadatajson, newerdata = stdout.split('\0', 1)
> +                    metadata.append(json.loads(metadatajson))
> +                except ValueError:
> +                    ui.warn(_('ignored invalid output from fixer tool: %s\n') %

Would the thing that processes this metadata usually not care to aggregate per fixer? If they did, it seems they would now have to look for a `fixer-applied='my-fixer'` entry and then take the metadata from the preceding entry. Did you consider instead making this function return an entry like `{fixername: metadatajson}` (and leave `metadatajson` as None if `not fixer.shouldoutputmetadata()`)?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6167

To: hooper, #hg-reviewers, durin42
Cc: martinvonz, lothiraldan, durin42, indygreg, mercurial-devel


More information about the Mercurial-devel mailing list