greglook / cljstyle

A tool for formatting Clojure code

Eclipse Public License 1.0

293 stars 39 forks source link

Keep strings on top of sorting #45

Closed wilkerlucio closed 3 years ago

wilkerlucio commented 4 years ago

Hello!

Today I want to bring a discussion and a change proposition to the library, I've been talking with @borkdude about the :unsorted-required-namespaces feature, there is a issue going on in borkdude/clj-kondo too https://github.com/borkdude/clj-kondo/issues/885.

The gist is about how to sort requires that include strings. Example:

; current-way
(:require
    [bar.core :as bar]
    ["caz" :as caz]))

; proposed way
(:require
    ["caz" :as caz]
    [bar.core :as bar]))

Other solutions like https://github.com/immoh/lein-nsorg or https://github.com/clojure-emacs/clojure-mode use the proposed way.

We had a guess that you used the first to follow clj-kondo conventions, but we are discussing if the second is not a better default.

What are you thoughts on it?

codecov-io commented 4 years ago

Codecov Report

Merging #45 into master will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #45   +/-   ##
=======================================
  Coverage   92.67%   92.67%           
=======================================
  Files          13       13           
  Lines        1515     1515           
  Branches       44       44           
=======================================
  Hits         1404     1404           
  Misses         67       67           
  Partials       44       44

Impacted Files	Coverage Δ
src/cljstyle/format/ns.clj	`96.64% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f62fe91...2931d8d. Read the comment docs.

filipesilva commented 3 years ago

Heya @greglook, just wanted to ping you about this change. Do you think this is the sort of thing that could get in, or maybe you prefer that some changes are made to the PR?

filipesilva commented 3 years ago

Heya @greglook, just a friendly ping about this change again, since clj-kondo is also doing strings on top currently. Is there anything I can do to help move this along?

greglook commented 3 years ago

Seems like this is the established convention. :+1: I might add a configuration option for this later.

filipesilva commented 3 years ago

Sweet, thanks for the merge!