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.

Sign up for GitHub

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

Jump to bottom

Fix specific highlighting (CMakeLists.txt ...) #7686

Merged
merged 4 commits into from
Aug 4, 2019
Merged

Fix specific highlighting (CMakeLists.txt ...) #7686

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
08f7333
Fix specific highlighting.
FlorianBen Jul 31, 2019
291bbea
Highlighting CMakeLists.txt:
FlorianBen Aug 2, 2019
964fcc7
Merge branch 'master' into master
lunny Aug 3, 2019
410dc7b
Merge branch 'master' into master
lunny Aug 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions modules/highlight/highlight.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ var (
"makefile": true,
}

// File names and extensions that are not same as highlight classes and may be case sensitive.
highlightSpecificNames = map[string]string{
"CMakeLists.txt": "cmake",
}

// Extensions that are same as highlight classes.
highlightExts = map[string]struct{}{
".arm": {},
Expand Down Expand Up @@ -82,6 +87,11 @@ func NewContext() {
// FileNameToHighlightClass returns the best match for highlight class name
// based on the rule of highlight.js.
func FileNameToHighlightClass(fname string) string {

if name, ok := highlightSpecificNames[fname]; ok {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should move these after ignore check.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that it does change anything as they are exclusively different maps

Copy link
Member

@sapk sapk Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Reviewing it I ask my self if we shouldn't replace highlightFileNames with map[string]string directly and only do one check.

Copy link
Member

@silverwind silverwind Aug 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, seems like this is duplicating with the purpose highlightFileNames, so I'd say lowercase the name and put it there instead. I guess in most cases we do want case-insensitive filename matching.

Copy link
Member

Choose a reason for hiding this comment

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

At least cmake requires CMakeFiles.txt file to be exactly like this named, so it is case-sensitive

Copy link
Member

Choose a reason for hiding this comment

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

I think this may also be true for Dockerfile and Makefile actually, for which we currently do that case-insensitve check, so I guess those could be moved to sensitive checks too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that make command works also with GNUmakefile and makefile filenames, whereas cmake only works with CMakeFiles.txt. GNUmakefile could be put in sensitive checks too.

Copy link
Member

Choose a reason for hiding this comment

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

By default, when make looks for the makefile, it tries the following names, in order: GNUmakefile, makefile and Makefile

We can lower-case all. I don't think it would be too bad to highlight cmakefiles.txt like CMakeFiles.txt. Futhermore, I don't know how it work with case-insensitive fs.

return name
}

fname = strings.ToLower(fname)
if ignoreFileNames[fname] {
return "nohighlight"
Expand Down