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
Add a VHDL checker using GHDL #1160
Conversation
@dzamlo Thanks… LGTM, but it will need a couple of refinements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a note about the new checker to CHANGES.rst
.
Could you perhaps also add a test case for the new syntax checker to test/flycheck-test.el
? Take a look at the other tests for syntax checkers which are at the end of that file…
:type '(choice (const :tag "Default standard" nil) | ||
(string :tag "Language standard")) | ||
:safe #'stringp | ||
:package-version '(flycheck . "0.31")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version's wrong here. We do not use a leading zero.
(flycheck-define-checker vhdl-ghdl | ||
"A VHDL syntax checker using GHDL." | ||
:command ("ghdl" | ||
"-s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's -s
for? Could you add comment to the source that explains the purpose of the flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is the flag to do the Syntax-check.
:command ("ghdl" | ||
"-s" | ||
(option "--std=" flycheck-ghdl-language-standard concat) | ||
source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can GHDL read the source code from standard input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No
252f120
to
64bdcc9
Compare
@dzamlo Thanks! Could you please squash all your commits into a single one so that we can merge? |
LGTM too! Thanks for working on this :) |
All my commits have been squashed. |
Is there something blocking this pull request ? I thinks that I addressed the requested changes. |
@dzamlo Terribly sorry for the delay. Your PR suffered a bad timing with the maintainer retiring. We'd still like to merge this! Can you rebase on master so we can proceed? |
Closing in favor of #1399. |
No description provided.