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

Attempting to add support for PHP #37

Closed
scwood opened this issue Oct 3, 2016 · 17 comments
Closed

Attempting to add support for PHP #37

scwood opened this issue Oct 3, 2016 · 17 comments
Labels
bug new tool Support for new linters, fixers, etc.

Comments

@scwood
Copy link

scwood commented Oct 3, 2016

Hello, I am trying to add support for php with php -l. I've added a directory called php to ale_linters and added a file called php.vim.

Here's my code so far:

if exists('g:loaded_ale_linters_php_php')
    finish
endif

let g:loaded_ale_linters_php_php = 1

function! ale_linters#php#php#Handle(buffer, lines)
    " Matches patterns like the following:
    "
    " Parse error: parse error in - on line 7
    let pattern = 'Parse error: \(.\+\) on line \(\d\+\)'
    let output = []

    for line in a:lines
        let l:match = matchlist(line, pattern)

        if len(l:match) == 0
            continue
        endif

        " vcol is Needed to indicate that the column is a character.
        call add(output, {
        \   'bufnr': a:buffer,
        \   'lnum': l:match[1] + 0,
        \   'vcol': 0,
        \   'col': 0,
        \   'text': l:match[0],
        \   'type': 'E',
        \   'nr': -1,
        \})
    endfor

    return output
endfunction

call ALEAddLinter('php', {
\   'name': 'php',
\   'executable': 'php',
\   'output_stream': 'stderr',
\   'command': 'php -l -',
\   'callback': 'ale_linters#php#php#Handle',
\})

When I run cat myfile.php | php -l I get sytnax errors. But in the buffer nothing shows up.

@w0rp
Copy link
Member

w0rp commented Oct 3, 2016

Hmm, the problem here isn't obvious to me yet. I might have a go at adding this to the repo myself and credit you as the author. It looks okay, so it's probably some minor bug.

@scwood
Copy link
Author

scwood commented Oct 3, 2016

Okay, let me know if there's something that needs to be changed. I would like to keep contributing. Great plugin.

@w0rp
Copy link
Member

w0rp commented Oct 3, 2016

I have pushed your code now with some minor changes. php needed -- instead of - to read from stdin, which is a bit odd, but it works. The second error was that the regex matches start at index 1, and not 0. That's a common mistake, which I didn't notice until I started playing around with it.

@w0rp w0rp closed this as completed Oct 3, 2016
@scwood
Copy link
Author

scwood commented Oct 3, 2016

Thanks for explaining those changes. I have the latest code, but I'm still
not seeing any linting feedback in my buffer.

Here's the code I'm using:

<?php
echo "Hello!" // this needs a semi-colon

I have pushed your code now with some minor changes. php needed --
instead of - to read from stdin, which is a bit odd, but it works. The
second error was that the regex matches start at index 1, and not 0.
That's a common mistake, which I didn't notice until I started playing
around with it.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#37 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AItA-uQz2tUR1g2lsaqevcls4XOqXFraks5qwY10gaJpZM4KNHXm
.

@w0rp
Copy link
Member

w0rp commented Oct 4, 2016

Ah, there is one bug left to be fixed which I can fix tomorrow. Some errors go one line beyond the last line in the file, so they don't appear at all. This is such an error. I'm thinking I might make that automatic for any linter which is added.

@w0rp
Copy link
Member

w0rp commented Oct 4, 2016

@scwood If you pull again, you should see a warning for the missing semicolon now. I fixed errors which have line numbers beyond the end of the file for any linter now.

@scwood
Copy link
Author

scwood commented Oct 4, 2016

I'm not sure what I'm doing wrong. I'm updated to the latest code. On the left with the C file Ale is working great, but on the php file nothing happens:

screen shot 2016-10-04 at 11 28 07 am

@w0rp
Copy link
Member

w0rp commented Oct 4, 2016

Hmm, could you paste the output of :!php -l -- < test.php and your php version? Maybe there is something wrong there.

I'll reopen the issue while you're having issues.

@w0rp w0rp reopened this Oct 4, 2016
@w0rp w0rp added bug new tool Support for new linters, fixers, etc. labels Oct 4, 2016
@scwood
Copy link
Author

scwood commented Oct 4, 2016

Sure, here's the ouput of :!php -l -- < test.php:

Parse error: parse error, expecting `','' or `';'' in - on line 4
Errors parsing -

shell returned 255

Press ENTER or type command to continue

And php:

$ php --version
PHP 5.5.36 (cli) (built: May 29 2016 01:07:06)
Copyright (c) 1997-2015 The PHP Group
Zend Engine v2.5.0, Copyright (c) 1998-2015 Zend Technologies

@deathmaz
Copy link
Contributor

deathmaz commented Oct 4, 2016

Hi, for me Ale works fine with php files:
https://asciinema.org/a/2h6zlyi99ialjdeia9gmiwteo

php -v

PHP 7.0.11 (cli) (built: Sep 16 2016 18:52:44) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies

@scwood
Copy link
Author

scwood commented Oct 4, 2016

Hmmm, I tried with Vim and NVim, no dice. Also tried with a minimal vimrc.

@w0rp
Copy link
Member

w0rp commented Oct 4, 2016

I'm not quite certain where the problem lies at the moment. I might try installing that version of PHP tomorrow and see if there are any problems there. The PHP linting works for me at the moment. Have you checked the linters for other languages? I wonder if the problem is specific to the PHP linter.

@scwood
Copy link
Author

scwood commented Oct 6, 2016

Yeah no issues using the other linters... I updated my plugin this morning. Still digging
screen shot 2016-10-05 at 7 50 02 pm

@scwood
Copy link
Author

scwood commented Oct 7, 2016

I have no idea what's going on with this. I've installed ale on my work computer and php works without any issues, I don't know why it's broken on my laptop. I've reinstalled vim, made sure I'm not running my forked version, etc. etc. I'm just going to close for now.

@scwood scwood closed this as completed Oct 7, 2016
@w0rp w0rp reopened this Oct 7, 2016
@w0rp
Copy link
Member

w0rp commented Oct 7, 2016

@scwood I think I finally figured out what was going on. I tried the PHP linter on a friend's machine, and it was outputting the errors to stdout, whereas my machine outputs them to stderr. I added support to the plugin for reading from both streams, so it will just pick up all lines of output and parse them. This might be one of those things you can change in php.ini. Let me know if it works for you now.

@scwood
Copy link
Author

scwood commented Oct 7, 2016

That did it, thanks so much.

@w0rp
Copy link
Member

w0rp commented Oct 7, 2016

Nice! You're welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug new tool Support for new linters, fixers, etc.
Projects
None yet
Development

No branches or pull requests

3 participants