Skip to content

Hanging attributes #55

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.

Sign up for GitHub

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

Open
AndrewRadev wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Hanging attributes #55

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4467c80
Check for a hanging attribute
AndrewRadev Jul 17, 2019
f17f6ba
Fix bug with component in-line
AndrewRadev Jul 17, 2019
a954d79
Save and restore state, just in case
AndrewRadev Jul 17, 2019
97fa57f
Merge branch 'master' into hanging-attributes
AndrewRadev Jul 25, 2019
87c3197
Fix bug with closing pattern after }} on the previous line
AndrewRadev Jul 25, 2019
e513ffb
Merge branch 'master' into hanging-attributes
AndrewRadev Dec 21, 2019
d03754a
Add an example of hanging attributes
AndrewRadev Aug 9, 2020
f2c0542
Merge branch 'master' into hanging-attributes
AndrewRadev Aug 9, 2020
a6d16d4
Fix unclosed <a> tag
AndrewRadev Aug 9, 2020
34356ba
Fix double comment
AndrewRadev Aug 9, 2020
79bf5f4
Move pattern out as a script-local var
AndrewRadev Aug 9, 2020
85df17c
Remove (hopefully) unnecessary check
AndrewRadev Aug 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions example.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
{{!-- Should not see unquoted "=", ".", or "/" as a mustacheOperator
unless inside mustacheInside or mustacheParam --}}
<a href=/index.html>
</a>

{{#repo}}
This is a mustache [enumerable] section
Expand Down Expand Up @@ -77,6 +78,10 @@
... with optional else added. Try matchit `%` command over if/else/if.
{{/if}}

{{component-with hanging="attributes"
over="several"
lines="."}}

Thanks goes to {{@defunkt}}
Feedback/blames go to {{@juvenn}}
{{Frustrations}} go to /dev/null
Expand Down
52 changes: 47 additions & 5 deletions indent/handlebars.vim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,58 @@ function! GetHandlebarsIndent(...)
" all indent rules only apply if the block opening/closing
" tag is on a separate line

" indent after block {{#block
if prevLine =~# '\v\s*\{\{[#^].*\s*'
" " check for a hanging attribute
Comment thread
AndrewRadev marked this conversation as resolved.
Outdated
let lastLnumCol = col([lnum, '$']) - 1
if synIDattr(synID(lnum, lastLnumCol, 1), "name") =~ '^mustache'
\ && prevLine !~# '}}\s*$'
let hangingAttributePattern = '{{[#^]\=\%(\k\|[/-]\)\+\s\+\zs\k\+='
let standaloneComponentPattern = '^\s*{{\%(\k\|[/-]\)\+\s*$'

if prevLine =~ hangingAttributePattern
" {{component attribute=value
" other=value}}
let [line, col] = searchpos(hangingAttributePattern, 'Wbn', lnum)
if line == lnum
return col - 1
endif
elseif prevLine =~ standaloneComponentPattern
" {{component
" attribute=value}}
return indent(lnum) + sw
endif
endif

let componentClosingPattern = '\v^\s*\{\{\/\S*\}\}\s*'
Comment thread
jsit marked this conversation as resolved.
Outdated

" check for a closing }}, indent according to the opening one

@jsit jsit Aug 9, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this part of hanging indent, too? Or is this a separate improvement? I just want to keep this PR as clean as possible.

@jsit jsit Aug 9, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now that this is necessary for the hanging indent. However, it looks like it duplicates some other functionality.

I notice that on this line:

https://github.com/mustache/vim-mustache-handlebars/pull/55/files#diff-eeee5d46b419bb17834d0b1a86a43f8fR114

It's returning indent(line) + sw, which is also happening later:

  " indent after block: {{#block, {{^block
  if prevLine =~# '\v\s*\{\{[#^].*\s*'
    let ind = ind + sw
  endif

Would it be possible to restrict this section to just the "hanging indent"-related conditionals?

@AndrewRadev AndrewRadev Aug 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the same case, though. The case that was already there applies on prevLine, while this new case is about the opening line of the prevLine.

Basically, the one that maches prevLine (the existing code) works for this case:

{{#foo}}
  [Cursor]
{{/foo}}

While the new code works on this case:

{{#foo one=two
       three=four}}
  [Cursor]
{{/foo}}

The previous line in this case is not the block opener, it's the three=four}} line. What's the indent on the next line from this one? Well, we need to find its opening line, which is the {{#foo line and take that one + one shiftwidth (if it's a block opener).

For a different explanation, consider the comments -- the existing code is indent after block: {{#block, {{^block. The new code is not indenting after a block tag, it's indenting after the closing part of any tag.

The only way I can see to unify them is to extend the new code to handle the old code's case, because "find the opening part" is the more generic logic. But trying it out, I seem to be breaking the {{else if case in the example, and I don't know why.

To be honest, I don't think it's worth the risk to try to combine the two cases, especially since it would make the one merged case more complicated and possibly trickier to change in the future. Let me know what you think and whether I've explained the difference between the cases well enough.

let saved_pos = getpos('.')
if prevLine =~# '}}$' && prevLine !~# '^\s*{{' &&
\ currentLine !~# componentClosingPattern &&
\ search('}}$', 'Wb')
let [line, col] = searchpairpos('{{', '', '}}', 'Wb')
if line > 0
if strpart(getline(line), col - 1, 3) =~ '{{[#^]'
" then it's a block component, indent a shiftwidth more
return indent(line) + sw
else
return indent(line)
endif
Comment on lines +112 to +117

@jsit jsit Aug 9, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if strpart(getline(line), col - 1, 3) =~ '{{[#^]'
" then it's a block component, indent a shiftwidth more
return indent(line) + sw
else
return indent(line)
endif
if strpart(getline(line), col - 1, 3) !=~ '{{[#^]'
return indent(line)
endif

We could almost do this, see what I mean? the indent(line) + sw for "normal" indents is happening later anyway, so it would be nice to keep that there.

else
call setpos('.', saved_pos)
endif
endif

" indent after block: {{#block, {{^block
if prevLine =~# '\v\s*\{\{[#^].*\s*' &&
\ prevLine !~# '{{[#^]\(.\{}\)\s.\{}}}.*{{\/\1}}'
Comment thread
AndrewRadev marked this conversation as resolved.
Outdated
let ind = ind + sw
endif
" but not if the block ends on the same line
if prevLine =~# '\v\s*\{\{\#(.+)(\s+|\}\}).*\{\{\/\1'
if prevLine =~# '\v\s*\{\{[#^](.+)(\s+|\}\}).*\{\{\/\1'
let ind = ind - sw
endif
" unindent after block close {{/block}}
if currentLine =~# '\v^\s*\{\{\/\S*\}\}\s*'
if currentLine =~# componentClosingPattern
let ind = ind - sw
endif
" indent after component block {{a-component
Expand All @@ -105,7 +147,7 @@ function! GetHandlebarsIndent(...)
let closingLnum = search('}}\s*$', 'Wbc', lnum)
let [openingLnum, col] = searchpairpos('{{', '', '}}', 'Wb')
if openingLnum > 0 && closingLnum > 0
if strpart(getline(openingLnum), col - 1, 3) !~ '{{[#^]'
if strpart(getline(openingLnum), col - 1, 3) !~# '{{[#^]'
let ind = ind - sw
endif
else
Expand Down