cyucelen / marker

🖍️ Marker is the easiest way to match and mark strings for colorful terminal outputs!

MIT License

47 stars 13 forks source link

Fix stamp overlap - MatchTimestamp #28

Closed marco-silveira closed 4 years ago

marco-silveira commented 4 years ago

Golang's regexp package, build under google's RE2 engine, doesn't support backtracking and look around assertions (https://github.com/golang/go/issues/18868). Since negative lookahead is the most straight forward solution, I've found this: https://github.com/dlclark/regexp2.

The only downside of using regexp2 (other than adding one more dependency to the project) is that it doesn't have constant time guarantees like the built-in regexp package, but since it's only used to Timestamp matches, I don't see it being a problem.

https://godoc.org/github.com/dlclark/regexp2 Ref #8

cyucelen commented 4 years ago

Could we just match for Jan _2 15:04:05(\s|$) and get rid of extra complexity stuff if possible. We can assume that no character comes after Stamp. We are marking the extra space in this case but, I believe simplicity is more important in this trade-off.

jnatalzia commented 4 years ago

+1 to the simplicity

marco-silveira commented 4 years ago

Could we just match for Jan _2 15:04:05(\s|$) and get rid of extra complexity stuff if possible. We can assume that no character comes after Stamp. We are marking the extra space in this case but, I believe simplicity is more important in this trade-off.

Sure, I think that's ok in these case too, but I don't know how to solve the overlaps with ANSIC and UnixDate layouts without using negative lookbehind. That's the main reason that made me use regexp2.

I will make the changes and update the PR, later on we can discuss the other overlaps @cyucelen

cyucelen commented 4 years ago

Of course @MarcoAntonioSilveira ! I feel like it will be a good challenge.

codecov-io commented 4 years ago

Codecov Report

Merging #28 into master will not change coverage. The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #28   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         121    121           
=====================================
  Hits          121    121

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 76d12a1...293e2d8. Read the comment docs.

marco-silveira commented 4 years ago

@cyucelen :recycle:

cyucelen commented 4 years ago

Is it ok to merge @MarcoAntonioSilveira ?

marco-silveira commented 4 years ago

@cyucelen yes