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

Merged
FlorianBen merged 4 commits from master into master 2019-08-04 08:11:28 +00:00
FlorianBen commented 2019-07-31 12:24:39 +00:00 (Migrated from github.com)

https://github.com/go-gitea/gitea/issues/7685
Add a new map for highlighting files that cannot be guessed with extension/filename (for instance, CMakeLists.txt).

https://github.com/go-gitea/gitea/issues/7685 Add a new map for highlighting files that cannot be guessed with extension/filename (for instance, CMakeLists.txt).
codecov-io commented 2019-07-31 12:43:54 +00:00 (Migrated from github.com)

Codecov Report

No coverage uploaded for pull request base (master@09463d1). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #7686   +/-   ##
=========================================
  Coverage          ?   41.28%           
=========================================
  Files             ?      472           
  Lines             ?    63845           
  Branches          ?        0           
=========================================
  Hits              ?    26360           
  Misses            ?    34045           
  Partials          ?     3440
Impacted Files Coverage Δ
modules/highlight/highlight.go 31.81% <0%> (ø)

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 09463d1...410dc7b. Read the comment docs.

# [Codecov](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=h1) Report > :exclamation: No coverage uploaded for pull request base (`master@09463d1`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit). > The diff coverage is `0%`. [![Impacted file tree graph](https://codecov.io/gh/go-gitea/gitea/pull/7686/graphs/tree.svg?width=650&token=t1G57YGbPy&height=150&src=pr)](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #7686 +/- ## ========================================= Coverage ? 41.28% ========================================= Files ? 472 Lines ? 63845 Branches ? 0 ========================================= Hits ? 26360 Misses ? 34045 Partials ? 3440 ``` | [Impacted Files](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=tree) | Coverage Δ | | |---|---|---| | [modules/highlight/highlight.go](https://codecov.io/gh/go-gitea/gitea/pull/7686/diff?src=pr&el=tree#diff-bW9kdWxlcy9oaWdobGlnaHQvaGlnaGxpZ2h0Lmdv) | `31.81% <0%> (ø)` | | ------ [Continue to review full report at Codecov](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=footer). Last update [09463d1...410dc7b](https://codecov.io/gh/go-gitea/gitea/pull/7686?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
lafriks (Migrated from github.com) approved these changes 2019-08-01 09:31:18 +00:00
lunny requested changes 2019-08-01 13:40:26 +00:00

Maybe we should move these after ignore check.

Maybe we should move these after ignore check.
lafriks (Migrated from github.com) reviewed 2019-08-01 14:04:06 +00:00
lafriks (Migrated from github.com) commented 2019-08-01 14:04:05 +00:00

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

I don't think that it does change anything as they are exclusively different maps
sapk (Migrated from github.com) reviewed 2019-08-01 14:48:27 +00:00
sapk (Migrated from github.com) commented 2019-08-01 14:48:27 +00:00

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

Reviewing it I ask my self if we shouldn't replace highlightFileNames with map[string]string directly and only do one check.
silverwind (Migrated from github.com) reviewed 2019-08-01 15:54:40 +00:00
silverwind (Migrated from github.com) commented 2019-08-01 15:54:40 +00:00

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.

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.
lafriks (Migrated from github.com) reviewed 2019-08-01 16:07:14 +00:00
lafriks (Migrated from github.com) commented 2019-08-01 16:07:14 +00:00

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

At least cmake requires CMakeFiles.txt file to be exactly like this named, so it is case-sensitive
silverwind (Migrated from github.com) reviewed 2019-08-01 16:11:37 +00:00
silverwind (Migrated from github.com) commented 2019-08-01 16:11:36 +00:00

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.

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.
FlorianBen (Migrated from github.com) reviewed 2019-08-02 08:22:44 +00:00
FlorianBen (Migrated from github.com) commented 2019-08-02 08:22:44 +00:00

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.

I think that `make` [command](https://www.gnu.org/software/make/manual/make.html#Makefile-Names) works also with `GNUmakefile` and `makefile` filenames, whereas `cmake` only works with `CMakeFiles.txt`. `GNUmakefile` could be put in sensitive checks too.
sapk (Migrated from github.com) reviewed 2019-08-02 08:30:10 +00:00
sapk (Migrated from github.com) commented 2019-08-02 08:30:10 +00:00

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.

> 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.
sapk (Migrated from github.com) approved these changes 2019-08-02 11:48:53 +00:00
sapk (Migrated from github.com) left a comment

Thanks for your first PR and your patience for the review process.

Thanks for your first PR and your patience for the review process.
lunny approved these changes 2019-08-03 03:08:19 +00:00
silverwind (Migrated from github.com) approved these changes 2019-08-04 07:31:11 +00:00
lafriks (Migrated from github.com) approved these changes 2019-08-04 07:50:38 +00:00
This repo is archived. You cannot comment on pull requests.
No Milestone
No project
No Assignees
1 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: lunny/gitea#7686
No description provided.