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.

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

Jump to bottom

Allow Haml 6.3 #459

Merged
merged 1 commit into from
Dec 13, 2023
Merged

Allow Haml 6.3 #459

merged 1 commit into from
Dec 13, 2023

Conversation

tagliala
Copy link
Contributor

Close #458

@tagliala tagliala force-pushed the feature/458-haml-63 branch from 1ec0641 to 74df25d Compare December 11, 2023 08:17
@tagliala tagliala force-pushed the feature/458-haml-63 branch from 74df25d to 0ea46fb Compare December 11, 2023 08:32
tagliala
tagliala commented Dec 11, 2023
View reviewed changes
lib/haml_lint/adapter.rb
Comment on lines 21 to +24
case version
when '~> 4.0' then HamlLint::Adapter::Haml4
when '~> 5.0', '~> 5.1', '~> 5.2' then HamlLint::Adapter::Haml5
when '~> 6.0', '~> 6.0.a', '~> 6.1', '~> 6.2' then HamlLint::Adapter::Haml6
when '~> 6.0', '~> 6.0.a', '~> 6.1', '~> 6.2', '~> 6.3' then HamlLint::Adapter::Haml6
Copy link
Contributor Author

@tagliala tagliala Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Should we change haml_version to haml_major_version, return segments.first.to_i, and go for:

      when 4 then HamlLint::Adapter::Haml4
      when 5 then HamlLint::Adapter::Haml5
      when 6 then HamlLint::Adapter::Haml6

or dynamic adapter resolution?

      adapter = "HamlLint::Adapter::Haml#{haml_major_version}".safe_constantize
      fail HamlLint::Exceptions::UnknownHamlVersion, "Cannot handle Haml version: #{haml_version}" unless adapter

      adapter

or any other suggestion.

The problem is that, as far as I can understand, it is possible that a single major version of Haml may require two different adapters, and then the dynamic resolution will fail

Maybe even drop Haml 4 and 5

Copy link
Owner

Choose a reason for hiding this comment

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

I'm supportive of dropping older versions of HAML, especially v4. I don't see a legitimate reason to continue supporting such an old version in light of how long v6 has been available.

@tagliala tagliala mentioned this pull request Dec 12, 2023
Merged
@sds sds merged commit 2befd56 into sds:main Dec 13, 2023
39 checks passed
@tagliala tagliala deleted the feature/458-haml-63 branch December 13, 2023 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sds sds sds left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow Haml 6.3
2 participants
@tagliala @sds