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

Flycheck doesn't recognize localized error messages from html-tidy #1376

Open
MaxLanar opened this issue Dec 21, 2017 · 20 comments
Open

Flycheck doesn't recognize localized error messages from html-tidy #1376

MaxLanar opened this issue Dec 21, 2017 · 20 comments

Comments

@MaxLanar
Copy link

  • Your Flycheck setup from M-x flycheck-verify-setup
    Syntax checkers for buffer index.html in html-mode:

    html-tidy

    • may enable: yes
    • executable: Found at /usr/bin/tidy
    • configuration file: Not found

Flycheck Mode is enabled. Use C-u C-c ! x to enable disabled
checkers.


Flycheck version: 32snapshot (package: 20171214.1215)
Emacs version: 24.5.1
System: x86_64-pc-linux-gnu
Window system: x

  • Your operating system
    debian stretch

  • Your Emacs version from M-x emacs-version
    24.5.1 by debian

Hello,
I've installed tidy from the debian repo, then with the last binary (5.4.0) provided by http://binaries.html-tidy.org/ then I compiled the 5.7.0 from source. Anyway, I always got those kind of errors:

Suspicious state from syntax checker html-tidy: Flycheck checker html-tidy returned non-zero exit code 1, but its output contained no errors: Ligne: 20 Col: 9 - Avertissement:absence de </div>

Try installing a more recent version of html-tidy, and please open a bug report if the issue persists in the latest release. Thanks!

And then sadly the checking do not work. (but for example it works OK with bash code with shellcheck)

I think I've done everything right, so I'm reporting the issue.
Let me know if I can do/check anything more.

@cpitclaudel
Copy link
Member

Looks like a translation issue to me. Do you know if there's a way to make your version of html-tidy print "Warning" instead of "Avertissement"?

@MaxLanar
Copy link
Author

I couldn't set 'language: en' in a config file to modify tidy's output, that option wasn't recognized (despite the man page saying it should be, it just triggered an error). I therefore manage to recompile tidy with localization support disabled.

Now it works properly with flycheck.

It is indeed a translation issue.

@cpitclaudel
Copy link
Member

Great, thanks for testing

@cpitclaudel
Copy link
Member

Maybe there's an environment variable we could set to force English?

@cpitclaudel
Copy link
Member

cpitclaudel commented Dec 21, 2017

But that will give untranslated messages, which is unfortunate. The best would be for tidy to give us structured output, like JSON or XML.

@fmdkdd
Copy link
Member

fmdkdd commented Dec 22, 2017

The best would be for tidy to give us structured output, like JSON or XML.

Agreed. If users want to see error messages in their own language, they should be able to do so.

@cpitclaudel
Copy link
Member

@MaxLanar , how motivated do you feel about this? :) Would you like to investigate?

@MaxLanar
Copy link
Author

I'm willing to help but I'm not sure I'm skilled enough for this... I'm not a (professional) dev at all (I sometimes play with html/css/bash, that's all). I'm not even sure I understand what need to be investigated. Can you be more specific ?

@cpitclaudel
Copy link
Member

Yes, of course :) And thanks! What would be great is to check whether we can make do with what html-tidy currently produces, or whether we'll have to ask them for a patch. To determine this, it'd be ideal to check the following:

  • Can html-tidy be made to print "info"/"warning"/"error" labels in English while using translated error messages?
  • Does html-tidy have (or is planning to have) support for alternate output formats (a common one is checkstyle; other checkers often use JSON)?

If the answer is no to both of these, the natural next step is to open a feature request with the html-tidy devs, requesting a JSON or XML output option, that we could use in Flycheck :)

Thanks!

@MaxLanar
Copy link
Author

I see, it's no big deal then ! I'll do it, no problem. :)

@cpitclaudel
Copy link
Member

Great, thanks a lot! Keep us posted :) We'd love Flycheck to work better in localized environments.

@MaxLanar
Copy link
Author

Okay, here is the results of my investigation :

  • Reading tidy's doc thoroughly, I couldn't find a way to make tidy print "info"/"warning"/"error" labels in English while using translated error messages.
  • I looked into tidy's github issues and PR with keywords like json, output, format, xml. I did the same with HTACG's public mailing lists. I found nothing related to our issue here. I visited HTACG's irc room but felt kinda lonely (the room was empty).

I'll drop a mail to the public mailing list, then, (if I got no answers,) open a feature request on their github (referencing our issue). Sounds good to you ?

As a workaround, waiting for a change from tidy, maybe flycheck could be made to add '-lang en' (that option doesn't work in a config file but do work as a parameter) to tidy's call ? That way non-english users could use flycheck without having to recompile tidy with localization disabled (and for example still have the benefit of being able to use tidy in their terminal and get output in their own language).

@cpitclaudel
Copy link
Member

Sounds perfect. Great work!
And yes, I'd like us to pass -lang. Would you like to prepare a PR?

@balthisar
Copy link

balthisar commented Apr 9, 2018

Gents, I'm here as a result of a message posted to the html-tidy dev list last night. I'm one of the Tidy developers (mostly in "offline" mode right now), but we're happy to entertain feature requests. Our roadmap is largely request driven, and so far, no one's made a request for JSON, etc.

A couple of things to understand, though, is that Tidy is two things: it's a C library that offers Tidy's services, but it's also a command line client that uses the library in interactive shells. Granted, over the years a lot of folks have adapted the command line client in a lot of ways that weren't really intended, given the C library and the ability of pretty much any language to work with C libraries.

When it comes to JSON, I'm probably the dev that would support it, and it wouldn't be horribly difficult to implement as a command line client option (not a configuration option), but I can't help but ask if the better solution wouldn't be for you to connect to the library directly? The library now offers an awesome callback-based API for collecting all of its output messages, and is designed to be language agnostic (i.e., the default English strings and localized strings are exposed).

In any case, feel free to open a feature request.

@fmdkdd
Copy link
Member

fmdkdd commented Apr 9, 2018

@balthisar Thanks for chiming in. I think calling the C library directly is unfortunately not an option for us. The only way to call the library, as far as I can tell, would be to write a dynamic module for Emacs that would call into the Tidy library. Emacs dynamic modules are a compile-time opt-in, so it would not work for most of our users who install Emacs via binary packages that do not include this option.

Good to hear you are open to requests then ;)

fmdkdd added a commit that referenced this issue Apr 9, 2018
Localized error messages also localize the diagnostic
type (e.g. "Warning" becomes "Avertissement" in French), which make them
unparseable by Flycheck.

Forcing English messages makes Flycheck parse the diagnostic correctly,
at the cost of losing the localization.

This is a partial fix for GH-1376.
@cpitclaudel
Copy link
Member

@balthisar Thanks for joining the discussion. We'd love to get JSON output.
That being said, it's some amount of work. An option to just only translate error messages (instead of error messages and labels) would work just as well for us :)

@balthisar
Copy link

Giving you JSON would actually be a bit easier than giving an option to only translate some of the strings. The string lookup service is kind of an all or nothing thing. I suppose we fake another language, e.g., French, but leave only the desired strings untranslated. This would be a horrible maintenance nightmare for us, though.

Would one of you mind filing a feature request over at our issues tracker? I can't promise high priority, but if it gets support, it can happen eventually, and if I get some time away from the newborn, I might actually be able to get it out fairly quickly.

@MaxLanar
Copy link
Author

I'll do it tonight !

@paxperscientiam
Copy link

Will this be fixed?

@fmdkdd fmdkdd changed the title Last tidy version doesn't work with flycheck. html-tidy Flycheck doesn't recognize localized error messages from html-tidy Nov 22, 2018
@fmdkdd
Copy link
Member

fmdkdd commented Nov 22, 2018

@paxperscientiam From the linked issue, it doesn't look like tidy has gotten JSON output support yet. If you want this fixed, I suggest helping out with that on tidy's side.

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