Skip to content

Implement multiline attributes #1043

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

Merged
merged 1 commit into from
Nov 2, 2020
Merged

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Oct 20, 2020

Fixes #981.

This is ported from Hamlit v1 code which supported multiline attributes. It was dropped when Hamlit started using Haml's parser in v2, but I'd be happy to resurrect the support. Please read the code comments for design decision details.

k0kubun added a commit to ruby/ruby that referenced this pull request Oct 20, 2020
This is a weird use case of Ripper.lex which I'm not sure is supposed to
be maintained, so I'm adding this test so that we can easily notice such
changes.

If we change the behavior, this will break the behavior of hamlit.gem v1
and code like haml/haml#1043.
@HamptonMakes
Copy link
Member

Whoa... I didn't see this! @k0kubun– you... you.... <3

@HamptonMakes HamptonMakes merged commit 000b665 into haml:main Nov 2, 2020
@k0kubun k0kubun deleted the multiline-attributes branch November 2, 2020 15:18
k0kubun added a commit to k0kubun/hamlit that referenced this pull request Dec 28, 2020
haml/haml#1043 was released at Haml 5.2.1.
k0kubun added a commit to k0kubun/hamlit that referenced this pull request Dec 28, 2020
haml/haml#1043 was released at Haml 5.2.1.
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jan 22, 2021
https://build.opensuse.org/request/show/865194
by user coolo + dimstar_suse
updated to version 5.2.1
 see installed CHANGELOG.md
  ## 5.2.1

  Released on November 30, 2020
  ([diff](haml/haml@v5.2.0...v5.2.1)).

  * Add in improved "multiline" support for attributes [#1043](haml/haml#1043)

  ## 5.2

  Released on September 28, 2020
  ([diff](haml/haml@v5.1.2...v5.2.0)).

  * Fix crash in the attribute optimizer when `#inspect` is overridden in TrueClass / FalseClass [#972](haml/haml#972)
  * Do not HTML-escape templates that are declared to be plaintext [#1014](haml/haml#1014) (Thanks [@cesarizu](https://github.com/cesarizu))
  * Class names are no longer ordered alphabetically, and now follow a new specification as laid ou
@kshnurov
Copy link

A simple %td{ title: 'asd' } asd'qwe fails with Old attributes must start with "{" and end with "}" because of that extra ' in the content. 5.2.0 works fine.
#1062

@k0kubun
Copy link
Member Author

k0kubun commented Jul 13, 2021

Sorry, I wasn't able to reproduce it. Could you prepare a repository that reproduces your issue? Thanks.

$ cat /tmp/a.haml
%td{ title: 'asd' } asd'qwe

$ ruby -v 
ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-darwin19]

$ haml -v
Haml 5.2.1

$ haml /tmp/a.haml
<td title='asd'>asd'qwe</td>

@kshnurov
Copy link

@k0kubun turns out it's a Ruby 2.4 issue. 2.5, 2.6, and 2.7 work fine.

$ cat ./a.html
%td{ title: 'asd' } asd'qwe

$ ruby -v
ruby 2.4.10p364 (2020-03-31 revision 67879) [x86_64-darwin18]

$ haml -v
Haml 5.2.1

$ haml ./a.html
Exception on line 219: Old attributes must start with "{" and end with "}"
  Use --trace for backtrace.

@k0kubun
Copy link
Member Author

k0kubun commented Jul 14, 2021

Yeah, it seems like a bug of Ripper (a part of Ruby). The problem is that the parser reorders tokens in edge cases like that, and it's a bit hard to implement a fix on Haml's side. Thus the bug report should go to ruby-core, instead of this haml repository.

$ ruby -v -rripper -e "puts Ripper.lex(%q[a( b: 'c' } d'e]).map{|x| x[2]}.join"
ruby 2.4.9p362 (2019-10-02 revision 67824) [x86_64-darwin18]
ea( b: 'c' } d'

$  ruby -v -rripper -e "puts Ripper.lex(%q[a( b: 'c' } d'e]).map{|x| x[2]}.join"
ruby 2.5.8p224 (2020-03-31 revision 67882) [x86_64-darwin19]
a( b: 'c' } d'e

JFYI, 2.3 also works fine. 2.2 and older have the bug, like 2.4.

$ ruby -v -rripper -e "puts Ripper.lex(%q[a( b: 'c' } d'e]).map{|x| x[2]}.join"
ruby 2.3.8p459 (2018-10-18 revision 65136) [x86_64-darwin18]
a( b: 'c' } d'e

@kshnurov
Copy link

@k0kubun 2.4 is EOL so there won't be a fix. Maybe it's time for HAML to drop Ruby 2.4- compatibility? @hcatlin

@HamptonMakes
Copy link
Member

@kshnurov what would that mean to you? We only test against Ruby 2.7+, and I wouldn't think we should error unless necessary.

@kshnurov
Copy link

@hcatlin README says Haml currently supports Ruby 2.0.0 and higher. I guess Ruby 2.4- users shouldn't be confused and upgrade to the latest versions of Haml.

@k0kubun
Copy link
Member Author

k0kubun commented Jul 14, 2021

We only test against Ruby 2.7+

Sorry for confusing you maybe by leaving a premature GitHub Actions config, but we currently test 2.5+ in Travis

haml/.travis.yml

Lines 9 to 10 in d3839d3

- 2.6
- 2.5

If we want to clarify the supported versions,

spec.required_ruby_version = '>= 2.0.0'
would be the best place to do so. (Of course we should update README as well like @kshnurov said)

@HamptonMakes
Copy link
Member

Thanks for the clarification everyone.

michaelzedwards added a commit to michaelzedwards/hamlit that referenced this pull request Feb 11, 2022
haml/haml#1043 was released at Haml 5.2.1.
sfo-den pushed a commit to sfo-den/hamlit that referenced this pull request Oct 7, 2024
haml/haml#1043 was released at Haml 5.2.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Multiline Attributes
3 participants