Skip to content

Conversation

@mikepjb
Copy link

@mikepjb mikepjb commented Dec 28, 2019

Hi Nate,

I've been trying to use the tailwindcss-language-server, the following snippet fixes it to work with vim-lsc.

The problem seems to be that the headers passed from the language server begins with an array e.g:

['[]
Content-Length: 184']

With the included if statement this gets removed, resulting in:

['Content-Length: 106']

I checked another server, typescript-language-server to make sure the change didn't break a previously working example.

I looked for some contributor guidelines but found none, I'd love to help get this working for other people.

@mikepjb
Copy link
Author

mikepjb commented Dec 30, 2019

I've updated this so we're not relying on the users ignorecase setting.

@natebosch
Copy link
Owner

This seems like a bug in the server but it's hard to know for sure. My reading of the spec doesn't allow the server to do this. The server doesn't appear to have source code published anywhere and there is no issue tracker I can find.

Thanks to a typo I can tell that it's using this code: https://github.com/microsoft/vscode-languageserver-node/blob/2d72432e59a83c45e6a8093ba51d097fa9ea7216/jsonrpc/src/messageWriter.ts#L67

I don't see anything there that looks like it would insert an arbitrary [], and the related parsing code doesn't seem to handle this either. https://github.com/microsoft/vscode-languageserver-node/blob/0a82a389aab66ced69c4578a002bc8d5f2c21386/jsonrpc/src/messageReader.ts#L65-L73

This does seem fairly safe, but it always makes me nervous to add a hack for a bug in some server.

Does this happen consistently for every message, or does it only happen occasionally? If it only happens on the first message, for instance, maybe there is some other initial behavior allowed by the spec that we should be accounting for elsewhere.

@natebosch
Copy link
Owner

What does your server config look like so I can try to reproduce this?

@natebosch
Copy link
Owner

After installing the server with npm and running it with the --stdio flag, I can't see any stray [] in the headers. If you can help me with some more detailed repro instructions I can look further.

@mikepjb
Copy link
Author

mikepjb commented Dec 30, 2019

Here's the config I'm using:

let g:lsc_server_commands = {
  \ 'html': {
  \   'command': 'tailwindcss-language-server --stdio',
  \   'suppress_stderr': v:true,
  \   },
  \ 'javascript': {
  \   'command': 'tailwindcss-language-server --stdio',
  \   'suppress_stderr': v:true,
  \   },
  \ }
let g:lsc_enable_autocomplete  = v:true

let g:lsc_auto_map = {
 \  'GoToDefinition': 'gd',
 \  'FindReferences': 'gr',
 \  'Rename': 'gR',
 \  'ShowHover': 'K',
 \  'Completion': 'omnifunc',
 \}

If I revert the change, autocomplete no longer works for tailwindcss-language-server

Without suppressing stderr I get the following messages:

[lsc:Error] StdErr from tailwindcss-language-server --stdio: Without `from` option PostCSS could generate wrong source m
ap and will not find Browserslist config. Set it to CSS file path or to `undefined` to prevent this warning.
[lsc:Error] Could not decode message: 
[lsc:Error] Error dispatching message: 'Vim(if):E715: Dictionary required

The [] at the beginning of headers seems pretty consistent, the project I'm reproducing this on is a 3 file style.css/index.html/plain tailwind.config.js so it's about as simple as can be.

Let me know if you need any more information, I'd rather not add hacks to fix a bug in another project either.

@mikepjb
Copy link
Author

mikepjb commented Dec 30, 2019

I can reproduce the issue using the first example from the tailwindcss screecasts github repo here:

https://github.com/tailwindcss/designing-with-tailwindcss/tree/master/01-getting-up-and-running/01-setting-up-tailwind-and-postcss

@natebosch
Copy link
Owner

Thanks for the specific example, I am able to see the [] when I run the server in that directory. It only shows up once at the beginning of the output. I traced it down in the minified code to a call to console.log(t.configDependencies) which indicates that it might not always be an empty array.

I don't think the fix here is the right thing to do - if we do anything at all it should probably be to handle errant content before the first message, rather than on every header. It should also handle a non-empty array - I don't know enough about the server to know if configDependencies will ever be non-empty.

I strongly suspect that this is an unintentional bug in the server - A console.log isn't appropriate for a server relying on stdout for other things. You probably want to follow up with the server author about this. Assuming it was something leftover from debugging it should be easy to remove. The best way to reach the author might be at https://github.com/bradlc/vscode-tailwindcss/issues

My inclination here is to avoid any changes in the plugin with the assumption that the server author will be willing to fix the bug. If that's not the case let me know and I can look again at how to handle it in a hopefully more robust way, perhaps by throwing away lines that end in "\n" but not "\r\n" until the first message. I'll close the PR for now, and we can reopen if the server author is not willing to fix the bug.

@natebosch natebosch closed this Dec 30, 2019
@natebosch
Copy link
Owner

If you're inclined to hack around this in another way, you could find the server file and edit it - it is minified, but you should be able to find and delete the console.log(t.configDependencies) call.

@mikepjb
Copy link
Author

mikepjb commented Dec 30, 2019

Thanks for your time and detailed feedback, I'll follow up with the author and let you know.

@mikepjb
Copy link
Author

mikepjb commented Jan 30, 2020

No news yet from tailwindlabs/tailwindcss-intellisense#91

Just letting you know I'm still chasing, hopefully will hear something soon 👍

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