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.

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

Passing FileName and LineNumber #57

Open
nsinklier opened this issue Jul 31, 2019 · 4 comments
Open

Passing FileName and LineNumber #57

nsinklier opened this issue Jul 31, 2019 · 4 comments
Assignees

Comments

@nsinklier
Copy link

nsinklier commented Jul 31, 2019

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
Copy link
Member

cnoon commented Jul 31, 2019

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?

@cnoon cnoon self-assigned this Jul 31, 2019
@nsinklier
Copy link
Author

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.

@indieSoftware
Copy link

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.

@indieSoftware
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants