Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support formatters that output a diff #42

Closed
dsedivec opened this issue Jul 3, 2021 · 2 comments
Closed

Support formatters that output a diff #42

dsedivec opened this issue Jul 3, 2021 · 2 comments

Comments

@dsedivec
Copy link

dsedivec commented Jul 3, 2021

Hi! Thank you for writing Apheleia!

I was integrating Apheleia with Darker yesterday, a Python reformatter. Darker is just Black, but with the key difference that Darker only reformats the portions of the file you've changed in your local Git repository. This is useful for me while I'm working in a legacy code base that is mostly unformatted today, but which aspires to eventually format everything with Black.

Darker really wants to be given the original input file, in place (so it can look at git diff I assume), and Apheleia can handle that just fine. However, Darker's only output options are:

  1. Update the file in place
  2. Output a diff

1 is not acceptable while you're editing the file, so we're left with 2.

Of course, a third option would be to modify Darker to support writing output to stdout or another file rather than updating the input file. However, I suspect there are other formatters that want to output a diff.

Since Apheleia is conveniently already operating on a diff, would you be interesting in adding support for formatters that output a diff?

I've actually hacked this up already—emphasis on "hacked"—and it seems to work fine in my very limited initial testing. I used el-patch (🔥) on apheleia-format-buffer to make a case where, if the first element of the formatter's command is rcs, the call to diff --rcs is simply skipped and we go straight to apheleia--apply-rcs-patch. Of course, Darker can only output unified diff format, so I wrote a little Python script to transform unified diff to RCS diff. I could port that unified → RCS diff format to Elisp easily enough.

I'd be happy to clean that up and submit a PR, but I won't bore you with it you're not interested.

Also, before I did rip off a PR, I wanted to check in to ask how you'd like the configuration for formatters to work? As I said above, I'm currently doing something like this in my hacky POC:

(defcustom apheleia-formatters
  '((darker . (rcs "darker" "--diff" filepath))
    ...) ...)

I think that's kind of ugly. It also looks too much like the existing npx semi-special value for my tastes.

Instead, my first choice would be to turn the values of the apheleia-formatters alist into alists of parameters, something like:

(defcustom apheleia-formatters
  '((black       . ((:command . ("black" "-"))))
    (brittany    . ((:command . ("brittany" file))))
    (darker      . ((:command . ("darker" "--diff" filepath))
                    (:output-format . unified-diff)))
    (prettier    . ((:command . (npx "prettier" "--stdin-filepath" filepath))))
    (gofmt       . ((:command . ("gofmt"))))
    (ocamlformat . ((:command . ("ocamlformat" file))))
    (terraform   . ((:command . ("terraform" "fmt" "-")))))
  ...)

This would allow for additional configuration parameters for formatters in the future, too. However (1) I don't know how you feel about breaking compatibility, and (2) I don't know if you like the aesthetics of that. Better suggestions very welcome!

@raxod502
Copy link
Member

Sorry for the delay, I've been on vacation for the past few weeks and am just catching up on emails now.

I'd be happy to clean that up and submit a PR

Yeah, this sounds good and in scope, would be happy to review and merge.

how you'd like the configuration for formatters to work

I think what you have currently is good. I agree it's somewhat odd, and we could consider redoing the whole interface to be more reasonable, but at least for now I think it's best to keep with the existing convention.

@dsedivec
Copy link
Author

Unfortunately I never got around to cleaning up my work on this, and then recent (good) changes to Apheleia broke the code I had for this. Finally, Darker now supports output to stdout, so I no longer have a need for directly interpreting diffs in Apheleia. I don't think I'll get back around to this again unless/until I have a need for it, and since I don't see any more indications that others need this, I'm going to go ahead and close this issue.

Thanks again for all your great Emacs work!

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

No branches or pull requests

2 participants