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

ns require indentation does not adhere to indentation rule 2 #87

Closed
imrekoszo opened this issue Sep 9, 2024 · 7 comments
Closed

ns require indentation does not adhere to indentation rule 2 #87

imrekoszo opened this issue Sep 9, 2024 · 7 comments
Labels
namespace related to ns form parsing or printing v1 blocker Required for a v1.0.0 release

Comments

@imrekoszo
Copy link
Contributor

imrekoszo commented Sep 9, 2024

(ns my.company.core (:require [clojure.string :as str]))

is currently (@chrisoakman/[email protected]) formatted to

(ns my.company.core
  (:require
    [clojure.string :as str]))

as opposed to

(ns my.company.core
  (:require
   [clojure.string :as str]))

Rule 2:

Other multi-line lists, vectors, maps and sets are aligned with the first element (1 or 2 spaces)

Edit: applies to other clauses as well:

(ns com.example.my-application.server
  "Example application HTTP server and routing."
  (:refer-clojure :exclude [atom count deref distinct empty first])
  (:require-macros
    [cljs.pprint :as pp])
  (:require
    [clojure.core.async :as async :refer [<! <!! >! >!!]]
    [com.example.my-application.base]
    [com.example.my-application.server.sse :as server.sse]
    [io.pedestal.http :as http]
    [io.pedestal.http.sse :as http.sse]
    [ring.util.response :as response])
  (:import
    (java.nio.file Files LinkOption)
    (org.apache.commons.io FileUtils))
  (:gen-class
    :init ctor
    :constructors undefined))
@NoahTheDuke
Copy link

I would expect that lists starting with keywords (function application in most cases) are treated as symbols for the purpose of indentation.

@imrekoszo
Copy link
Contributor Author

I would expect that lists starting with keywords (function application in most cases) are treated as symbols for the purpose of indentation.

I don't have a very strong opinion about this, it's just that my understanding of @tonsky's rule 1 is that it's only about symbols.

@oakmac
Copy link
Owner

oakmac commented Sep 10, 2024

I can see this either way. I think I prefer the 2 space indentation as it creates a stronger indentation impression, especially with reader conditionals.

The good news is: as the ns form is always "printed from scratch" it will always be consistent regardless of the initial input format.

@imrekoszo
Copy link
Contributor Author

@oakmac are you looking to apply that 2 space indentation to other lists-starting-with-a-keyword as well, i.e. treating keywords as symbols for this purpose as Noah suggests, or are you looking to only do 2 spaces for the ones in the ns form? My personal preference would be consistency between the two.

@tonsky
Copy link

tonsky commented Sep 11, 2024

My 2 cents:

  • Original idea was to have all lists indented with 2 spaces and all vectors/maps with one
  • Because parinfer would be confused with something like
  ((abc)
    def)

we had to make exception (1 space) for when first place is list/vector/map.

So now we have a situation:

  • Lists with symbol on first place — definitely 2 spaces
  • Lists with list/vector/map on first place — definitely 1 space

But the rest is up to intepretation.

One can make a case that then reader macro will look off:

#?(:clj ...
    :cljs ...)

Also lists when they are just lists:

'(1
   2
   3)

So it looks like any list with non-symbol in first place should be 1-space indented?

@oakmac
Copy link
Owner

oakmac commented Oct 18, 2024

Some thoughts on this:

  • Either way will work fine, due to ns forms being "printed from scratch" and always consistent.
  • I think it is ok if the ns form has it's own rules for indentation.
    • ie: it does not have to be beholden to how indentation work elsewhere

My personal preference is two-space indentation (how it works now), but I am leaning toward one-space indentation in order to match how to ns. This project has taken a lot of guidance from those recommendations, so I think it makes sense to follow it here.

It also seems like one-space indentation is far more common than two space:

@oakmac oakmac added v1 blocker Required for a v1.0.0 release namespace related to ns form parsing or printing labels Oct 18, 2024
@oakmac
Copy link
Owner

oakmac commented Oct 18, 2024

PR-130 adjusts ns indentation to match closer with the dominant community style

@oakmac oakmac closed this as completed Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
namespace related to ns form parsing or printing v1 blocker Required for a v1.0.0 release
Projects
None yet
Development

No branches or pull requests

4 participants