-
Notifications
You must be signed in to change notification settings - Fork 349
Generate 'stack' tool style compilation targets. #1494
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
Conversation
Looks good to me. If nobody raises any voices agaist I'll merge this in a day or two. |
What are "stack tool style" compilation targets exactly btw? Is this any different from cabal's compilation target syntax? For reference (as I couldn't find a place where this is documented), Cabal uses the following (fully-qualified) syntax to identify components within a project containing one or more packages:
where When it's unambiguous, it's allowed to use less qualified forms, e.g.
|
haskell-cabal.el
Outdated
(push projectName matches) | ||
(push val matches)))) | ||
(push (if (eq 'stack-ghci process-type) | ||
(concat projectName ":" val) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is a projectName
? Does Stack name projects?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
projectName
is whatever appears in the 'name:' field at the top of the cabal file, it's in one of the hidden sections in the diff, but projectName
is defined on line 481 with (haskell-cabal--get-field "name")
. Stack doesn't have a separate notion of a project to what cabal provides as far as I can tell.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I see, then clearly projectName
is a misnomer (but your PR didn't introduce it... it was already in the codebase before that... :-) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can switch it to package-name
which might be more appropriate while I'm making edits tonight.
I hadn't realized that cabal also uses that syntax since as you said it isn't documented anywhere. Stack uses a very similar syntax, but it's required: https://docs.haskellstack.org/en/stable/build_command/#target-syntax |
Is cabal syntax same as stack syntax? If so we should just be using fully qualified target specs for all cases. This is autocompletion after all, so it makes sense to list fully qualified target names. @hvr: good catch, I did not know cabal has extended syntax. Since when is it supported? |
@gracjan Comparing the description and Stack's documentation, the one case where it seems to be different is that Stack allows something like |
We should do fully qualified syntax always. @csasarak: can you fix that? Thanks. |
@gracjan I will try to take care of that tonight. |
@gracjan I think the 3-part syntax is only supported in |
@hvr Is it okay then to assume that current cabal users will use |
@csasarak: I'm not using |
@ivan-m Good to know, I'm playing with stack and cabal tonight (I'm most familiar with stack) so I can take a more educated stab at this. |
So unfortunately the acceptable syntax for targets are different between stack and cabal. I have a solution that I want to test a bit more, but if we anticipate stack command syntax deviating from cabal more in the future (or vice versa) it would probably make more sense to forgo my solution and have some abstraction so that stack command and cabal command strings aren't being generated in the same place. |
@csasarak I think it's reasonable to assume that Stack won't coordinate their syntax w/ Cabal upstream. So anticipating syntax deviation and abstracting seems reasonable. |
Ebal has a very nice interface for this. |
ccc8c35
to
2dc3765
Compare
For uninitiated: https://github.com/mrkkrp/ebal |
In this case we should split code paths inside haskell-mode for ghci, cabal and stack properly. This is a more involved change, but needed very much. Would be great to have a more principled approach to the issue. |
I've added some tests to my original implementation for the stack/cabal fully-qualified targets. I'd need some time to look at the code in order to make a decent decision about how to split things up and will be traveling for the next week and a half or so, so I won't make any progress during that time. Still, if you have any advice for where to start ( |
We can merge it for now and refactor later. Is it ready to be merged in current state? |
To the best of my knowledge it's ready to be merged. |
This is an initial attempt at addressing issue #1187. Please let me know if there's a better way, this is my first outing into the haskell-mode code.