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

Stylish haskell after save #1137

Merged
merged 2 commits into from
Feb 6, 2016
Merged

Conversation

bergey
Copy link
Contributor

@bergey bergey commented Feb 6, 2016

Fixes #1074. Fixes #788. Fixes #256.

@bergey
Copy link
Contributor Author

bergey commented Feb 6, 2016

Whoops. Force-pushed parenthesis fix.

gracjan added a commit that referenced this pull request Feb 6, 2016
@gracjan gracjan merged commit 438b914 into haskell:master Feb 6, 2016
@gracjan
Copy link
Contributor

gracjan commented Feb 6, 2016

Nice triple kill! Thanks.

@wraithm
Copy link
Contributor

wraithm commented Feb 8, 2016

I updated to the newest melpa version of haskell-mode today. With haskell-stylish-on-save t in my emacs config, haskell-mode reverts changes that I made on a save. The Messages buffer only says,

Saving file /path/to/my/haskell/File.hs...
Wrote /path/to/my/haskell/File.hs

If I comment out my haskell-stylish-on-save t, everything works again.

@gracjan
Copy link
Contributor

gracjan commented Feb 8, 2016

Ha! See here: https://github.com/haskell/haskell-mode/blob/master/haskell-commands.el#L824

         (_errcode (with-temp-file tmp-file
             (call-process cmd filename
                   (list (current-buffer) err-file) nil)))

Although there is a temp file generated, it is not used, the original filename is used and stylish-haskell replaces its contents.

@gracjan
Copy link
Contributor

gracjan commented Feb 8, 2016

@bergey: can you fix this? Thanks.

@gracjan
Copy link
Contributor

gracjan commented Feb 8, 2016

(note: I've reverted the change for now).

@bergey
Copy link
Contributor Author

bergey commented Feb 8, 2016

Sorry about that. Thanks for the diagnosis and reverting for now. I'll fix it, probably next weekend.

@gracjan
Copy link
Contributor

gracjan commented Feb 9, 2016

Great!

@wraithm
Copy link
Contributor

wraithm commented Feb 9, 2016

Thank you both for all of your hard work :)

@gracjan
Copy link
Contributor

gracjan commented Feb 9, 2016

@wraithm, actually can you help us with this? You have the commits by @bergey and I've pointed you to the problem of using original file name instead of temporary file name. Can you try to fix this?

Helpful info how to pull a pull request is here: https://help.github.com/articles/checking-out-pull-requests-locally/

Can you give it a try?

@wraithm
Copy link
Contributor

wraithm commented Feb 10, 2016

Yeah! I'm down to try. I'm a novice in elisp, but this is a good excuse to learn.

What would you recommend for setting up a development environment? Like, I have the melpa version of haskell-mode installed right now. Is there a good way to tell emacs to load up my fork instead? I've cloned my fork in my home directory (ie. ~/haskell-mode)

@wraithm
Copy link
Contributor

wraithm commented Feb 10, 2016

Nevermind. I think I figured it out. I just added my fork to the load-path, and that seemed to do it.

@gracjan
Copy link
Contributor

gracjan commented Feb 10, 2016

Yes, load-path is the way to go!

@gracjan
Copy link
Contributor

gracjan commented Mar 31, 2016

@wraithm, @bergey: Is this moving forward?

@bergey
Copy link
Contributor Author

bergey commented Mar 31, 2016

Sorry, I got busy with other projects, and don't foresee getting back to this.

@wraithm
Copy link
Contributor

wraithm commented Apr 5, 2016

Yeah, I'm sorry :( I similarly got distracted with work. I'm going to work on this now.

@wraithm
Copy link
Contributor

wraithm commented Apr 6, 2016

Okay, so I got it working, but errors from stylish-haskell get dumped into the buffer...

@wraithm
Copy link
Contributor

wraithm commented Apr 6, 2016

Making a PR now so you guys can look at it

@wraithm
Copy link
Contributor

wraithm commented Apr 6, 2016

#1266

@wraithm
Copy link
Contributor

wraithm commented Apr 6, 2016

#1268

Okay!

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

Successfully merging this pull request may close these issues.

None yet

3 participants