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

Upgrade language server version to 0.0.21 #1078

Merged
merged 1 commit into from
Jun 21, 2019

Conversation

rcjsuen
Copy link
Contributor

@rcjsuen rcjsuen commented Jun 21, 2019

This new update is mainly focused around improvements to the validation to correct some false positives (fixes #952) in addition to a fix to how variables were being resolved. Now the language server only considers alphanumeric and underscore characters as part of a variable's name.

As always, please use the following Dockerfile before and after the pull request to verify the changes:

# VOLUME fixes -------------------------------------------------
FROM node
# hover over VOLUME, there is a typo "specifid" that is now fixed
VOLUME [ "/data" ]
# invoke Intellisense here, the documentation has the same typo "specifid" that is now fixed
VOL

# RUN instruction linting fixes --------------------------------
FROM scratch
# there was an error on the question mark, this is now fixed
RUN echo ${bbb:?}

# there was an error on the "T", this is now fixed
# see https://github.com/microsoft/vscode-docker/issues/952
RUN echo ${Env:Temp}

# FROM linting fixes -------------------------------------------
# there was no error before, now this will be properly flagged as being an invalid base image
FROM $image

# variable resolution linting fixes ----------------------------
FROM scratch
ARG ARG_VAR=1234
ENV ENV_VAR $ARG_VAR
# "$ENV_VAR" was incorrectly highlighted as an error, now it's okay
EXPOSE "$ENV_VAR"

FROM scratch
ARG ARG_VAR=1234
# "$ARG_VAR" was incorrectly highlighted as an error, now it's okay
EXPOSE "$ARG_VAR"

# variable resolution fixes ------------------------------------
FROM alpine
ENV var value
# put your cursor in $var, the whole $var:test gets highlighted
# now it only highlights $var
RUN echo $var:test

FROM alpine
ENV var value
# put your cursor in $var, hit F2 to rename, it used to
# show var:test, now it only shows var
RUN echo $var:test

FROM alpine
ENV var value
# put your cursor in $var, hit F2 to rename, rename the variable,
# only the RUN gets modified, the ENV declaration should get modified also
RUN echo $var:test

FROM alpine
ENV var value
# put your cursor in $var, hit F12 to open the definition,
# nothing happens, now it will jump to the variable declaration
RUN echo $var:test

FROM alpine
ENV var value
# hover your cursor over $var, nothing is shown, now it resolve correctly
# and show "value"
RUN echo $var:test

This update includes some fixes to the linting checks so that there
are less false positives. It also includes a fix to how variables
are resolved to ensure that only alphanumeric and underscore
characters are considered.

Signed-off-by: Remy Suen <[email protected]>
Copy link
Contributor

@ejizba ejizba left a comment

Choose a reason for hiding this comment

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

Looks good - thanks!

@ejizba ejizba merged commit 147125d into microsoft:master Jun 21, 2019
@rcjsuen rcjsuen deleted the lsp-0.0.21 branch June 21, 2019 20:15
@rcjsuen
Copy link
Contributor Author

rcjsuen commented Jun 21, 2019

@EricJizbaMSFT Thanks for reviewing this! Have a nice weekend!

@bverkron
Copy link

Trying to track down the source of my issue and the trail has led me here. Is the following something that should be solved by this PR or is it something else?

image

@rcjsuen
Copy link
Contributor Author

rcjsuen commented Oct 10, 2019

Trying to track down the source of my issue and the trail has led me here. Is the following something that should be solved by this PR or is it something else?

image

@bverkron Thank you for reaching out. Not exactly, your issue stems from your use of the \ character which is the default escape character within a Dockerfile. You should define a different escape character when working with Windows paths.

Check out the Dockerfile reference for more details.

@bverkron
Copy link

Fair point, but should the syntax highlighting / error checking recognize the escape command and adjust accordingly? It does not seem to.

@rcjsuen
Copy link
Contributor Author

rcjsuen commented Oct 10, 2019

Fair point, but should the syntax highlighting / error checking recognize the escape command and adjust accordingly? It does not seem to.

The linting feature should recognize your parser directive if you have defined one. I suggest you open a new issue if you are still seeing issues.

As to the syntax highlighting, the language server is not doing the calculations there. See #75 and #292.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update language server version for fix: Dockerfile linting problems with variables in RUN instructions
3 participants