haskell-cabal-subsection-arrange-lines inserts superfluous commas

[bergmark]

If I C-c C-s a section of exposed modules this happens:

  Exposed-Modules:
    Data.Label
    Data.Label.Base
    Data.Label.Derive
    Data.Label.Failing
    Data.Label.Monadic
    Data.Label.Mono
    Data.Label.Partial
    Data.Label.Point
    Data.Label.Poly
    Data.Label.Total

=>

  Exposed-Modules:
                    Data.Label
                  , Data.Label.Base
                  , Data.Label.Derive
                  , Data.Label.Failing
                  , Data.Label.Monadic
                  , Data.Label.Mono
                  , Data.Label.Partial
                  , Data.Label.Point
                  , Data.Label.Poly
                  , Data.Label.Total

This isn't wrong, but it's also unnecessary to insert commas under exposed-modules, data-files, extra-source-files, and possibly others. The only time I know the comma is necessary is in build-depends since individual constraints can contain spaces.

[bergmark]

Additionally, if i run the same command on e.g. the ghc-options field the output depends on whether it contains a comma (maybe this is the case for all sub-sections),

  GHC-Options: -Wall -Werror
=>
  GHC-Options: -Wall -Werror

and

  GHC-Options: -Wall, -Werror
=>
  GHC-Options: -Wall
             , -Werror

[gracjan]

@bergmark: you have ventured into an interesting territory. Thanks for the report. We need to wait on volunteer that will fix this issue.

[geraldus]

@bergmark thank you! I missed this functionally for ages (but haven't a chance to find a time to put my hands on this question), didn't know that this is already exists.

[mrBliss]

I would like to have a go at this.

Some remarks:

• I suppose this behaviour should be configurable. I propose to add a defcustom haskell-cabal-comma-separated-fields (in haskell-cabal.el) which is a list of strings (or symbols?) that indicate which sections should be separated by comma's when calling haskell-cabal-subsection-arrange-lines. bergmark can then remove "exposed-modules", "data-files", ... from this list.
• Should this haskell-cabal-comma-separated-fields list be populated with all the relevant sections by default or just by the ones that actually need commas? The advantage of the former is that the default behaviour does not change.

[gracjan]

Great!

Please keep section style:

• if a section had commas before C-c C-s, then it should be formatted with commas
• if a section did not have commas before C-c C-s, then it should be formatted with spaces

No defcustom required.

Note that this functionality needs to properly support quoted items in lists. Do not forget about ERT tests.

[mrBliss]

Thanks for the hints! Good idea to just adhere to the current style.

I had already begun with first writing the ERT tests 😃

[gracjan]

Nice.

It seems that C-c C-s is hardcoded to use only commas-before mode. Should we care about the commas-after, too?

[mrBliss]

It's not really hardcoded because haskell-cabal.el contains the following code:

(defcustom haskell-cabal-list-comma-position
  'before
  "Where to put the comma in lists"
  :safe t
  :group 'haskell-cabal
  :type '(choice (const before)
                 (const after)))

However, I don't think many people know of this option. An alternative is to remove it altogether and to just detect the used style. I'll have a go at this too.

[mrBliss]

What about a single line with comma-separated options, e.g., ghc-options:? It would be unclear whether the user intends to use commas before or trailing commas.

I think it would be better in this case to just keep them on a single line. If the user wants them on separate lines, he can always insert a single newline at the right place, and then call haskell-cabal-subsection-arrange-lines to fix up the rest.

[gracjan]

Yes, defcustoms are evil and should be eradicated.