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

Remove Dockerfile_ file glob from Docker lexer #1550

Merged
merged 1 commit into from
Jul 4, 2020

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Jun 19, 2020

In addition to adding the *.Dockerfile file glob, #1059 added a file glob for Dockerfile_* to address #1070. However, as noted by #1539, this causes issues for files that begin with dockerfile_ but that are not Dockerfiles.

While I am the one that merged #1059, I now believe it was a mistake to add the Dockerfile_* file glob and it should be removed. This is a non-standard name and is not recognised by other lexing libraries (see Pygments, Chroma, Linguist). I was not aware of this feature at the time but the correct fix for the issue that precipitated #1070 is to use a .gitattributes file as explained here.

While this is technically a breaking change and should wait for the Rouge 4.0 release, I think it makes sense to characterise it as a bug that can be fixed as part of the next release of Rouge 3.0.

This fixes #1539.

@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Jun 19, 2020
@pyrmont pyrmont self-assigned this Jun 19, 2020
@pyrmont
Copy link
Contributor Author

pyrmont commented Jun 19, 2020

@sethbergman As described in the OP, it was my mistake not to suggest using .gitattributes as the solution to your request. I'm sorry that this PR will cause you additional work :(

@sethbergman
Copy link

@sethbergman As described in the OP, it was my mistake not to suggest using .gitattributes as the solution to your request. I'm sorry that this PR will cause you additional work :(

It's not a problem, we're already using a different format for Dockerfile anyway, so I have implemented the .gitattributes strategy.

@pyrmont pyrmont merged commit 9872cee into rouge-ruby:master Jul 4, 2020
@pyrmont pyrmont deleted the bugfix.docker-greedy-fileglobs branch July 4, 2020 05:58
@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Jul 4, 2020
@pyrmont
Copy link
Contributor Author

pyrmont commented Jul 4, 2020

This change has now been merged into master and will be part of v3.21.0. That is scheduled for release on Tuesday 14 July.

mattt pushed a commit to NSHipster/rouge that referenced this pull request May 19, 2021
In addition to adding the `*.Dockerfile` file glob, rouge-ruby#1059 added a file
glob for `Dockerfile_*` to address rouge-ruby#1070. However, as noted by rouge-ruby#1539,
this causes issues for files that begin with `dockerfile_` but that are
not Dockerfiles.

Adding the `Dockerfile_*` file glob was an error and this commit
removes it. It is a non-standard name and is not recognised by other
syntax highlighting libraries (e.g. Pygments, Chroma, Linguist). The
correct fix for the issue that precipitated rouge-ruby#1070 is to use a
`.gitattributes` file.
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.

Guesser: dockerfile_spec.rb is considered Ambiguoss instead of ruby
2 participants