Skip to content

Update tmLanguage file to cover first escape character#36766

Merged
thaJeztah merged 1 commit intomoby:masterfrom
Myeongjoon:dev_moby
Oct 27, 2024
Merged

Update tmLanguage file to cover first escape character#36766
thaJeztah merged 1 commit intomoby:masterfrom
Myeongjoon:dev_moby

Conversation

@Myeongjoon
Copy link
Contributor

@Myeongjoon Myeongjoon commented Apr 2, 2018

- What I did
fixes #36695.

- How I did it
make duplicated escape code to variable and use it outside of begin,end statement.

- How to verify it
test this code.
This is in #36695

FROM ubunto:16.06 

RUN echo //blep.derp.com:9875/:_authToken=\'er99iWoLDerYl666'YRUS134YfE'ghohhrrrr516LwfM=\' >> ~/.npmrc
RUN echo //blep.derp.com:9875/:_authToken=\"er99iWoLDerYl666YRUS134YfEghohhrrrr516LwfM=\" >> ~/.npmrc
RUN echo //blep.derp.com:9875/:'_authToken=\'er99iWoLDerYl666YRUS134YfEghohhrrrr516LwfM=\' >> ~/.np'mrc
RUN echo //blep.derp.com:9875/:_aut"hToken=\'er99iWoLDerYl666Y"RUS134YfEghohhrrrr516LwfM=\' >> ~/.npmrc

test_code

- Description for the changelog

updated tmLanguage file

- A picture of a cute animal (not mandatory but encouraged)
cat

Signed-off-by: Myeongjooon Kim kimmj8409@gmail.com

@GordonTheTurtle GordonTheTurtle added dco/no Automatically set by a bot when one of the commits lacks proper signature status/0-triage labels Apr 2, 2018
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Apr 2, 2018
@moby moby deleted a comment from GordonTheTurtle Apr 3, 2018
@Myeongjoon Myeongjoon mentioned this pull request Apr 4, 2018
Closed
@derek derek bot added the rebuild/* label Dec 21, 2018
@codecov
Copy link

codecov bot commented Dec 21, 2018

Codecov Report

Merging #36766 into master will increase coverage by 2.18%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #36766      +/-   ##
==========================================
+ Coverage   34.93%   37.11%   +2.18%     
==========================================
  Files         614      646      +32     
  Lines       45645    58640   +12995     
==========================================
+ Hits        15944    21765    +5821     
- Misses      27607    34015    +6408     
- Partials     2094     2860     +766
Impacted Files Coverage Δ
pkg/mount/mounter_linux.go 68.57% <0%> (-20.91%) ⬇️
daemon/metrics.go 33.07% <0%> (-8.39%) ⬇️
daemon/logger/logger.go 73.8% <0%> (-8.34%) ⬇️
daemon/cluster/executor/container/validate.go 92.59% <0%> (-7.41%) ⬇️
daemon/top_unix.go 62.79% <0%> (-5.96%) ⬇️
pkg/sysinfo/sysinfo.go 76.31% <0%> (-5.51%) ⬇️
daemon/config/config_common_unix.go 64.28% <0%> (-4.95%) ⬇️
client/ping.go 82.35% <0%> (-4.32%) ⬇️
pkg/parsers/parsers.go 96.22% <0%> (-3.78%) ⬇️
client/container_start.go 75% <0%> (-2.78%) ⬇️
... and 223 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7410b04...9bce9ca. Read the comment docs.

@tcd
Copy link

tcd commented Oct 15, 2019

Correct me if I'm wrong, but I don't think any changes to the reported coverage were caused by changes made the commit in this pull request?

@tcd
Copy link

tcd commented Oct 19, 2019

Also, this might help to clarify what issue is being addressed.

This is how that snippet is currently highlighted:

before

And this is how it would be highlighted with the new definition that properly recognizes escaped quotes:

after

The second image is a more accurate representation of the code, as strings in a Dockerfile are not delimited by \' or \".
The current highlighting seems to imply that they are.

@thaJeztah thaJeztah mentioned this pull request Apr 21, 2020
Open
thaJeztah
thaJeztah reviewed Oct 25, 2024
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

Stumbled on this PR, and looks like it was never merged; let me give it a quick rebase and bring it in

contrib/syntax/textmate/Docker.tmbundle/Syntaxes/Dockerfile.tmLanguage Outdated
Comment on lines +11 to +16
<key>repository</key>
<dict>
<key>string-character-escape</key>
<dict>
<key>name</key>
<string>constant.character.escaped.dockerfile</string>
Copy link
Member

@thaJeztah thaJeztah Oct 25, 2024

Choose a reason for hiding this comment

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

Looks like there's mixed tabs/whitespaces here

@thaJeztah thaJeztah added this to the 28.0.0 milestone Oct 25, 2024
@Myeongjoon @thaJeztah
Update tmLanguage file to cover first escape character
c2029cb 
If matched escape character like \" before enter " in begin statement, text editor return invalid Highlighting.
So included escape character outside of begin statement and did some refactoring.
Signed-off-by: Myeongjooon Kim kimmj8409@gmail.com

Signed-off-by: Myeongjoon Kim <kimmj8409@gmail.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
thaJeztah approved these changes Oct 25, 2024
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah added kind/bugfix PR's that fix bugs area/builder Build labels Oct 25, 2024
@thaJeztah thaJeztah merged commit b5f0008 into moby:master Oct 27, 2024
@thaJeztah thaJeztah mentioned this pull request Aug 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees

@johnstep johnstep

Labels

area/builder Build area/contrib kind/bugfix PR's that fix bugs status/4-merge

Projects

None yet

Milestone

28.0.0

Development

Successfully merging this pull request may close these issues.

Problem with syntaxic coloring of quotes in a Dockerfile

6 participants

@Myeongjoon @tcd @thaJeztah @AkihiroSuda @johnstep @GordonTheTurtle