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

Run sass-lint from the target's directory #1573

Merged
merged 1 commit into from May 25, 2018
Merged

Conversation

zed0
Copy link
Contributor

@zed0 zed0 commented May 11, 2018

Running sass-lint from the directory of the target file causes the local linter config to be used (if it exists) rather than the global config.

  • If you add or modify a function for computing a command line string for
    running a command, please add Vader tests for that.

Can someone point me in the direction of a Vader test that does this already? I'm not familiar with vimscript so I'll have a very hard time without something to follow.

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Look at the test directory for examples of tests.

@@ -3,6 +3,6 @@
call ale#linter#Define('sass', {
\ 'name': 'sasslint',
\ 'executable': 'sass-lint',
\ 'command': 'sass-lint -v -q -f compact %t',
Copy link
Member

Choose a reason for hiding this comment

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

That won't work on Windows. Look at how other linters use BufferCdString. grep for it.

@w0rp
Copy link
Member

w0rp commented May 15, 2018

I just added this sentence to the pull request template, which should help you.

Look at existing tests in the test/command_callback directory, etc.

@zed0
Copy link
Contributor Author

zed0 commented May 15, 2018

I've added a basic test and changed how the directory is changed as suggested.
I'm not sure how to have tests for both versions of this linter and maintain the naming scheme for the tests given they are both called sasslint.

Copy link
Member

@w0rp w0rp left a comment

Choose a reason for hiding this comment

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

Looks good. I'll move that function over to its own file and use the same one in both places, most likely.

@w0rp w0rp merged commit 8d49da1 into dense-analysis:master May 25, 2018
@w0rp
Copy link
Member

w0rp commented May 25, 2018

Cheers! 🍻

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.

None yet

2 participants