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

Show format for --help flag. Fixes #2460 #2607

Closed
wants to merge 1 commit into from
Closed

Show format for --help flag. Fixes #2460 #2607

wants to merge 1 commit into from

Conversation

psibi
Copy link

@psibi psibi commented May 24, 2015

Fixes #2460

@23Skidoo
Copy link
Member

It's intentionally hidden because it's somewhat buggy.

@gbaz
Copy link
Collaborator

gbaz commented May 29, 2015

Now that #2353 is closed, are there any other blockers on exposing this?

@bgamari
Copy link
Contributor

bgamari commented Jun 10, 2015

@23Skidoo?

@23Skidoo
Copy link
Member

@bgamari Sorry, I'm sick right now, will answer in a few days.

@bgamari
Copy link
Contributor

bgamari commented Jun 16, 2015

@23Skidoo ping (apologies if you are still ill).

@dcoutts
Copy link
Contributor

dcoutts commented Jun 18, 2015

Patch looks fine, assuming we're now happy with exposing this feature.

@23Skidoo
Copy link
Member

Well, the patch itself is trivial, but format has been a constant source of bugs, so I'd like to see some more testing done before merging. One suggestion is to go through all .cabal files on Hackage and check that readPackageDescription path is equivalent to (readPackageDescription path >>= writeGenericPackageDescription path) >> readPackageDescription path. Looking through open tickets I see #2661, maybe there is more. Other enhancements that'd be nice to see: #1966, #1127.

@phadej
Copy link
Collaborator

phadej commented Jun 24, 2015

Simple check for @23Skidoo read . write . read = read, using https://github.com/commercialhaskell/all-cabal-files

module Main (main) where

import Control.Monad
import Control.Applicative
import Distribution.Verbosity
import Distribution.PackageDescription.Parse
import Distribution.PackageDescription.PrettyPrint
import Distribution.ParseUtils

hoist :: ParseResult a -> Maybe a
hoist (ParseOk _ x) = Just x
hoist (ParseFailed _) = Nothing

check :: FilePath -> IO ()
check cabal = do
  print cabal
  desc <- readPackageDescription silent cabal
  let desc' = parsePackageDescription (showGenericPackageDescription desc)
  when (Just desc /= hoist desc') $ do
    print $ Just desc
    print $ hoist desc'
    fail cabal

main :: IO ()
main = do
  -- Generate listing with
  -- find . -type f -name '*.cabal' > cabal-files
  cabalFiles <- lines <$> readFile "cabal-files"
  mapM_ check cabalFiles

Fails already at ./accelerate-cuda/0.12.0.0/accelerate-cuda.cabal on my machine (using Cabal in master)

@23Skidoo
Copy link
Member

@phadej Thanks! I think we shouldn't expose format until this test passes for all .cabal files on Hackage.

@23Skidoo
Copy link
Member

Current status: blocked by #2661 and #2681.

@BardurArantsson
Copy link
Collaborator

I'm sure this is nice'n'all, but I'm not sure "round-tripping" is the be-all-and-end-all of package description formats[1]. So, I'd argue to just say WONTFIX... as unpopular as that may be. I'd rather argue that the cmdline arg and all supporting code should be removed.

(I hope I haven't completely misunderstood the point of this PR.)

[1] Parsers of said formats, sure, but not the semantics.

@m-renaud
Copy link
Collaborator

m-renaud commented Apr 1, 2020

I have recently started taking a closer look at the cabal format command and there are quite a few changes that we need to make before this is generally available. I have started tracking the work at https://github.com/haskell/cabal/projects/9, expect some updates there in the coming weeks.

I'm going to close out this PR since it'll be a little while before we want this to be published.

@m-renaud m-renaud closed this Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cabal format is not listed in --help message
9 participants