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

Feature request: enforce a consistent attributes wrapper style #89

Open
cannikin opened this issue Sep 14, 2018 · 12 comments
Open

Feature request: enforce a consistent attributes wrapper style #89

cannikin opened this issue Sep 14, 2018 · 12 comments

Comments

@cannikin
Copy link

https://www.rubydoc.info/gems/slim/frames#Attributes_wrapper

Maybe with a config option like:

AttributesWrapper:
  enabled: true
  style: none|any|curly|round|square

If I can figure out how the slim parser identifies these perhaps I can submit a PR soon. :)

@cannikin
Copy link
Author

cannikin commented Sep 14, 2018

Hmmm, I'm close... how can I set a different config option in my spec? I'm trying to test the different style settings but I don't see anyone else settings those on the fly in their specs, they're all just using config/default.yml

@cannikin
Copy link
Author

I think I got it! Within a context block:

let(:config) do
  SlimLint::ConfigurationLoader.load_hash({
    'linters' => {
      'AttributesWrapper' => {
        'enabled' => true,
        'style' => 'round'
      }
    }
  }).for_linter(described_class)
end

@cannikin
Copy link
Author

@sds Is there any way, when matching with on, that you can get the whole line that contains that match?

The tree for these two lines is identical as far as the parser is concerned:

div foo="bar"
div(foo="bar")

So inside an on block I can't tell any difference between them. What I'd really love to do is match any HTML element with attributes, then inspect the whole line with a regex to determine if there are matching opening/closing brackets (or no brackets at all).

My 90% solution uses on_start and just inspects each line with a regex, but it doesn't like ones that contain no attributes at all:

div Foo

And I haven't even got into what happens inside an embedded engine block like javascript:

So ideally I'd only bother inspecting lines that actually contained an HTML element with attributes and go from there. But I can't see a way to get the whole line's content when matching with on...

@Darhazer
Copy link

@cannikin it seems (from looking at the source code) that this is done with document.source_lines

on [...] do |sexp|
  line = document.source_lines[sexp.line() - 1]

@cannikin
Copy link
Author

cannikin commented Oct 9, 2018

I solved my problems above by using an on block and then only inspecting lines that the parser identifies as HTML with attributes. Great!

But I'm stuck on one last use case: attributes that span more than one line with line breaks. I can't figure out the best matcher to use to get back a sexp that can somehow tell me how many lines a statement is spanning.

Any advice would be appreciated!

@chevinbrown
Copy link

I'd like to do exactly this. @cannikin did you ever get your linter working? Would you be willing to share what you have so far?

@cannikin
Copy link
Author

@chevinbrown Mine is here, I don't think I ever got it to handle attribute spread over multiple lines though :( I've been in React-land for the past 6 months so I haven't used it in a while!

https://github.com/cannikin/slim-lint/tree/attributes-wrapper

@chevinbrown
Copy link

@cannikin thanks!

@chevinbrown
Copy link

@cannikin FWIW, my solution was nearly identical to yours, but I just checked for either opening or closing style. If they don't match, another cop/parser will pick that up.
It's still 90%, so my best-guess would be:
Grab the last line and reverse-concat them back to the first...or somehow rebuild the sexp & parse until match... 🤷‍♂
I'll keep iterating.

@dkniffin
Copy link
Contributor

@chevinbrown @cannikin Any progress on finishing this out? I'd love to start enforcing a consistent attributes style in my Slim code.

@chevinbrown
Copy link

@dkniffin I didn't get much further than this: https://github.com/cannikin/slim-lint/blob/attributes-wrapper/lib/slim_lint/linter/attributes_wrapper.rb

@cannikin
Copy link
Author

Sorry, I haven't used this at all since my last post, I haven't been able to write slim in years!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants