-
Notifications
You must be signed in to change notification settings - Fork 917
Added file, function and line information to logging in debug builds. #9437
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR extends the File Provider logging stack to capture and (in debug builds) emit source location context (file, function, line) alongside existing structured log details.
Changes:
- Extended
FileProviderLogging.write(...)andFileProviderLogger.{debug,info,error,fault}to thread throughfile/function/line. - Added
file/function/linefields toFileProviderLogMessageand encoded them only in#if DEBUG. - Appended source location to unified logging output (debug builds) and adjusted some path interpolations to use explicit
.publicprivacy.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.../Log/FileProviderLogging.swift |
Updates the logging protocol API to carry source location info. |
.../Log/FileProviderLogger.swift |
Passes caller #filePath/#function/#line through the logger convenience methods. |
.../Log/FileProviderLogMessage.swift |
Adds source fields to the JSON log model and conditionally encodes them in debug builds. |
.../Log/FileProviderLog.swift |
Threads source location through log writing and appends it to unified logs (debug). |
Comments suppressed due to low confidence (1)
shell_integration/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLog.swift:219
- When
detailsis empty you early-return after logging only the message, so the newfile:function:linecontext is omitted for those log entries. To match the PR’s intent, include the source location even when there are no details (e.g., log message + "file:line function" in the empty-details branch too).
if details.isEmpty {
logger.log(level: level, "\(message, privacy: .public)")
return
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...cOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogging.swift
Show resolved
Hide resolved
...cOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogging.swift
Outdated
Show resolved
Hide resolved
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
...X/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogMessage.swift
Show resolved
Hide resolved
...X/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogMessage.swift
Outdated
Show resolved
Hide resolved
d6072da to
f5dd749
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...X/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogMessage.swift
Show resolved
Hide resolved
...n/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLog.swift
Outdated
Show resolved
Hide resolved
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
…ging in debug builds. Also fixed some unnecessary masking. Signed-off-by: Iva Horn <iva.horn@nextcloud.com>
f5dd749 to
a31b12f
Compare
|
Artifact containing the AppImage: nextcloud-appimage-pr-9437.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...acOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogger.swift
Show resolved
Hide resolved
...n/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLog.swift
Show resolved
Hide resolved
...n/MacOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLog.swift
Show resolved
Hide resolved
...cOSX/NextcloudFileProviderKit/Sources/NextcloudFileProviderKit/Log/FileProviderLogging.swift
Show resolved
Hide resolved
|



No description provided.