OyvindSabo / gremlint

Linter/Code formatter for Gremlin
https://gremlint.com
Apache License 2.0
11 stars 6 forks source link

When non-Gremlin code blocks are left unformated, their total length should not be used to determine if wrapping is necessary #65

Closed OyvindSabo closed 3 years ago

OyvindSabo commented 3 years ago

Consider that we want to format the following query, with a max line length of 45:

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(map({ it.get().value('factor_a') * 
             it.get().value('factor_b') }))

It should be formatted to

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(map({ it.get().value('factor_a') * 
             it.get().value('factor_b') }))

but instead it is formatted to

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(
      map(
{ it.get().value('factor_a') * 
             it.get().value('factor_b') }))

even if it is just 45 characters wide (but it is formatted correctly if we give it 86 characters)

OyvindSabo commented 3 years ago

The problem here is that when we determine if an expression should be wrapped, we look at the length of the expression if formatted to a singel line, and then we start by wrapping the top level code of that expression, then the first nested layer and then the next, etc. SInce I have basically settled on the fact that the code formatter will primarily focus on Gremlin-code, I think we can make the simplification that lambda closures can be formatted similar to regular steps, i.e. not with vertical alignment of code, but with level-based indentation, producing something like

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(
      map({
        it.get().value('factor_a') *
        it.get().value('factor_b') }))

instead of

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(map({ it.get().value('factor_a') * 
             it.get().value('factor_b') }))

Editi: I think maybe the second of the two approaches here is the best. The formatter shouldn't try to make as few assumptions about the non-Gremlin code as possible, so just keeping the formatting of non-Gremlin code exactly as provided seems most sensible. The only think which should be done is to align it with the surrounding code, but keep relative indentation between each line of the non-Gremlin code. Then, to determine if the code should wrap, we can simply look at the longest line of the non-Gremlin code block.

OyvindSabo commented 3 years ago

And it should work similarly if the curly brackets are not wrapped in parentheses:

g.V(ids).
  has('factor_a').
  has('factor_b').
  project('Factor A', 'Factor B', 'Product').
    by(values('factor_a')).
    by(values('factor_b')).
    by(
      map{
        it.get().value('factor_a') *
        it.get().value('factor_b') })