Passing FileName and LineNumber

[nsinklier]

Willow/Source/Logger.swift

Line 147 in 816ff01

open func error(_ message: @autoclosure @escaping () -> LogMessage) {

I don't see a way to attach the file name and line number to the logs, which would be useful for errors especially. Is this something you've considered and discarded or just something you haven't needed? I'm going to add it to my project, if it's something you're interested in, I'd be happy to make a PR at add this. This is basically the code:

// function
    open func error(_ message: @autoclosure @escaping () -> LogMessage, _ file: String = #file, _ line: Int = #line) {
        print("file = \(file)\nline = \(line)")
        logMessage(message, with: LogLevel.error)
    }

// function call (unchanged)
log.error(Message.requestFailed(request: request, response: fakeResponse, error: nil))

// resulting output
file = /Users/.../Desktop/Willow-5.1.0 2/Example/Frameworks/Database/Connection.swift
line = 63
2019-07-31 12:27:57.744 💣💥💣💥 [Database] => SQL Error: ["error_description": "The database file is locked", "framework_name": "Database", "result": "failure", "framework_version": "1.0", "sql": "CREATE TABLE cars(make TEXT NOT NULL, model TEXT NOT NULL)", "error_code": 5]

This would be especially helpful when creating a LogWriter to post logs to a server. That way you can determine error locations based on logs.

[cnoon]

Hi @nsinklier,

This is certainly something we've been experimenting with as well and have decided to place it on top of Willow itself for now. If we wanted to do this directly in Willow, we'd probably want to support file, function, line, and column in each of the Logger APIs which would then forward those to the LogWriter APIs. That would allow writers to decide how to use them if they wanted.

Given the scope of this change, I'm not sure it's something you'd actually like to take on yourself since it would be a MAJOR version change. It's certainly something that we've been noodling on for a while.

Thoughts?

[nsinklier]

Hey @cnoon, thanks for the quick response! I definitely agree that it should be attached to every LogLevel, I just didn't want to suggest something so invasive outright :)

I was thinking that since I'm going to make these changes anyway, I could just send you the code to look at. Then, if you think it would be beneficial we could incorporate it, otherwise I'll just keep it for myself. I've only spent about an hour and it's almost done, so I don't think the change will be too time consuming, it's just a lot of lines of code changed.

I personally think that the position belongs to a log message. Without the position, log messages are only half as valuable. That's why I implemented this feature request and opened a pull request.

Hey @cnoon, any plans to merge PR #60 in the next time?