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

[WIP] Better Logger #6901

Closed
wants to merge 10 commits into from
Closed

[WIP] Better Logger #6901

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
48eaeb8
remove old logger
Oct 1, 2018
80e94c3
add new logger
Oct 1, 2018
fcaf32f
update playground to use new logger
Oct 2, 2018
9618593
remove lazily-evaluated messages
ezrast Oct 10, 2018
ecc2c0e
change emitters to array
ezrast Oct 10, 2018
37f14e6
merge Base into Logger; use Logger::ROOT as the root instead of Logger
ezrast Oct 10, 2018
14d176d
replace Severity::UNKNOWN with Severity::SILENT
ezrast Oct 10, 2018
2305179
add root_filter and root_emitter methods
ezrast Oct 10, 2018
f81ab7c
split out Dispatcher class from Logger; make Logger a thin struct
ezrast Oct 21, 2018
c6c4a17
update playground for new logger
ezrast Oct 21, 2018
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
52 changes: 23 additions & 29 deletions src/logger.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,29 @@ class Logger
alias FilterType = Filter | Severity | (Entry -> Bool)
alias EmitterType = Emitter | (Entry -> Nil)

module Base
abstract def component : String
abstract def filter : FilterType?
abstract def emitters : Array(EmitterType)

def log(entry : Entry)
case ff = filter
when Filter, Proc(Entry, Bool)
return unless ff.call(entry)
when Severity
return unless entry.severity >= ff
end
emitters.each &.call(entry)
property component : String
property filter : FilterType?
property emitters : Array(EmitterType)
Copy link
Member

Choose a reason for hiding this comment

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

Why did the ivar accessors became properties? I don't think they need to be mutable. getter should be enough.

Copy link
Author

Choose a reason for hiding this comment

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

I made filter a property since Logger::ROOT.filter = ... needs to be possible, and then brought the rest along for consistency. But I agree immutability would be better in other cases. I think that's an argument in favor of making Logger.get return a class that is thinner than Logger.

component can be made immutable, in any case. emitters doesn't really matter since the array will always be mutable.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, I see the last commit. Yeah, when we're not using thin wrappers, filter should probably be mutable. But component and emitters do not.


def log(entry : Entry)
case ff = filter
when Filter, Proc(Entry, Bool)
return unless ff.call(entry)
when Severity
return unless entry.severity >= ff
Copy link
Contributor

Choose a reason for hiding this comment

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

You mean return if entry.severity < ff

Copy link
Member

Choose a reason for hiding this comment

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

Why? The expressions are semantically equal.

Copy link
Contributor

Choose a reason for hiding this comment

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

"if X is inferior than Y" is simpler to understand than "unless X is superior or equal than Y", my opinion.

end

{% for level in Severity.constants %}
def {{ level.downcase.id }}(message, *, time = Time.now, line_number = __LINE__, filename = __FILE__)
log Entry.new(message, Severity::{{ level }}, component, time, line_number, filename)
end
{% end %}
emitters.each &.call(entry)
end

include Base
getter component : String
getter filter : FilterType?
getter emitters : Array(EmitterType)
{% for level in Severity.constants %}
def {{ level.downcase.id }}(message, *, time = Time.now, line_number = __LINE__, filename = __FILE__)
log Entry.new(message, Severity::{{ level }}, component, time, line_number, filename)
end
{% end %}

def get(component) : Logger
Logger.new(component.to_s, nil, Forwarder.new(self))
end

def initialize(@component, @filter, @emitters)
end
Expand All @@ -41,13 +38,10 @@ class Logger
new(component, filter, [emitter] of EmitterType)
end

extend Base
class_property component = ""
class_property filter : FilterType?
class_property emitters : Array(EmitterType) = [IOEmitter.new] of EmitterType
ROOT = new("", nil, IOEmitter.new)

def self.get(component)
Logger.new(component.to_s, nil, Forwarder.new(self))
def self.get(component) : Logger
ROOT.get component
end

{% for level in Severity.constants %}
Expand Down
2 changes: 1 addition & 1 deletion src/logger/emitter.cr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Logger
class Forwarder
include Emitter

getter dest : Base
getter dest : Logger

def initialize(@dest)
end
Expand Down