Skip to content

(work in progress) Add support for write-good linter#811

Closed
sumnerevans wants to merge 8 commits into
dense-analysis:masterfrom
sumnerevans:write-good-linter
Closed

(work in progress) Add support for write-good linter#811
sumnerevans wants to merge 8 commits into
dense-analysis:masterfrom
sumnerevans:write-good-linter

Conversation

@sumnerevans

@sumnerevans sumnerevans commented Jul 28, 2017

Copy link
Copy Markdown
Contributor

Resolves #643.

This is a work in progress, but I wanted some feedback on what I have so far, and I wanted to get an answer to a question (below).

Things that I know still need to be done:

  • Vader tests
  • Documentation of options for write-good linter

Question:
Just like with proselint, the write-good linter can be applied to many file types. The problem is that, unlike proselint, the write-good linter allows the user to specify some options (more along the lines of the eslint linter).

Looking at the documentation, it appears that if a linter exposes options to the user, it needs to be documented in the Integration Documentation under the appropriate language(s). This is a problem, since write-good applies to multiple languages but does not have any configuration that changes across languages (this is unlike gcc, for example, which has different options for C/C++).

Should I duplicate the documentation for all of the languages for which the write-good linter applies? It seems messy (if anyone changes anything in the future, they'd have to edit multiple files), but at the same time, it seems to be most consistent with the current documentation structure.

@w0rp

w0rp commented Jul 28, 2017

Copy link
Copy Markdown
Member

If the options are to be configured the same for every instances of write-good at once, then create a single option for write-good, document the option in the global configuration settings, and link to the help tag for the global option from the write-good heading for each language.

@w0rp

w0rp commented Jul 28, 2017

Copy link
Copy Markdown
Member
===============================================================================
write-good                                                *ale-html-write-good*

See |ale-write-good-options|

You can do that.

@w0rp w0rp added this to the Version 1.6 milestone Aug 28, 2017

@w0rp w0rp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is looking pretty good. See my comments here.

Now you can go and write the tests for the functions.

Comment thread doc/ale.txt Outdated


-------------------------------------------------------------------------------
3.2. Options for write-good Linter *ale-write-good-options*

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should either write "options for write-good," or "options for the write-good linter." The latter sounds weird without the definite article. The former sounds better.

Comment thread doc/ale.txt
3.2. Options for write-good Linter *ale-write-good-options*

The options for the write-good linter are global because it does not make
sense to have them specified on a per-language basis.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This explanation doesn't quite sound right. Instead of making excuses for why this is different, explain the advantages of configuring everything in one place instead.

Comment thread autoload/ale/handlers/writegood.vim Outdated
\ . ' %t'
endfunction

function! s:HandleWriteGoodFormat(buffer, lines, type) abort

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can move the contents of this function into the one below.

@w0rp w0rp modified the milestones: Version 1.6, Version 1.7 Oct 22, 2017
@w0rp

w0rp commented Oct 24, 2017

Copy link
Copy Markdown
Member

I just rebased this, squashed it, added tests for it, slapped your name on it, and merged it. It's now finally in there. 👍

@w0rp w0rp closed this Oct 24, 2017
@sumnerevans

Copy link
Copy Markdown
Contributor Author

@w0rp , sorry. Thanks for picking this up. I started this over the summer and then school hit. :)

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.

2 participants