guibou / PyF

Haskell QuasiQuoter for String Formatting
BSD 3-Clause "New" or "Revised" License
66 stars 13 forks source link

Remove haskell-src-exts dependency #51

Closed carlosdagos closed 3 years ago

carlosdagos commented 4 years ago

Draft PR as to explore an alternative to #49. Would also perhaps help solve #9, #8, and maybe #16.

Draft because it's really hacked together :smile:

Things to note:

I will add: Thank you so much for adding such a thorough dev environment with Nix, and comprehensive tests. It makes contributing so much nicer.

Carlos

guibou commented 4 years ago

Thank you so much.

Let me have a look at that. That's really great!

nix-build -A ormolu-fix should actually be a nix-shell, because it uses a shellHook to run ormolu in the local directory instead of running it on the source moved to /nix/store ;) I should make that more obvious in the documentation or even in nix itself.

guibou commented 4 years ago

I've added a small review. I really don't have a lot to say, that's already in pretty good shape.

However I'm really embarrassed by the next step. The point of removing haskell-src-ext and haskell-src-meta is to:

So are the benefit of your work outweighting the downside?

guibou commented 4 years ago

I will add: Thank you so much for adding such a thorough dev environment with Nix, and comprehensive tests. It makes contributing so much nicer.

Thank you about that. To be honest, I'm ashamed of the quality of PyF code (have you seen the well named function fofo? ;), so my only hope to get something workable was to add tests and tooling.

guibou commented 4 years ago

I'm adding that using the ghc lib is, from my point of view, perfectly fine here. The code which we will parse is part of the current codebase, so I think it is safe to assume that a code compiler with GHC 8.6 won't have GHC 8.10 expression in its PyF expression.

luke-clifton commented 4 years ago

@carlosdagos do you plan on pushing this further?

guibou commented 4 years ago

@luke-clifton I may have been a bit rough with @carlosdagos and I apologize for that.

There are still open questions, mostly:

carlosdagos commented 4 years ago

@guibou so sorry that I never got back to this! You weren’t rough at all, your review was thorough and I appreciate that. Apologies for the radio silence, some private matters took up my time.

I think a “meta” package would be nice in general, but might take a bit longer. I know that the haskell-src-exts package is deprecated but I don’t know the “urgency” around not using it anymore.

guibou commented 3 years ago

Closed in favor of #61 which is exactly the same pull request, but at the time of opening the PR, I did not realized that it will be a new one instead of continuing on this one.

In short, I'd like to merge it like that (with the minor changes I did in #61), we'll see later how to improve (and release) Meta.hs.