Skip to content

Conversation

@khassel
Copy link
Collaborator

@khassel khassel commented Oct 21, 2025

follow up to discussions in #3923

This small change adds the calling filename (path und suffix are stripped) to the log messages coming from server side, example screenshot:

grafik

I choosed grey as color for better readability.

Let me know if this change is wanted and if the duplicates in the default module log messages should stay or removed.

Links:

@sdetweil
Copy link
Collaborator

Not sure the MagicMirror code prefix (app,server…) is useful, and certainly not double
99% are not as convoluted as calendar and newsfeed

@khassel
Copy link
Collaborator Author

khassel commented Oct 21, 2025

Not sure the MagicMirror code prefix (app,server…) is useful

but do they hurt? I prefer to stay with this instead of implementing filters for removing.

and certainly not double 99% are not as convoluted as calendar and newsfeed

yes, they should be removed. We could also think of renaming some files so it is clearer to what they belong, e.g. git-helper

@sdetweil
Copy link
Collaborator

Doesn’t hurt.

@KristjanESPERANTO
Copy link
Collaborator

Let me know if this change is wanted

I like it 👍

and if the duplicates in the default module log messages should stay or removed.

You mean entries like ... [node_helper] [nodehelper] ...?

Removed.


What effect does that have on 3rd-party-modules?

@khassel
Copy link
Collaborator Author

khassel commented Oct 21, 2025

You mean entries like ... [node_helper] [nodehelper] ...?

yes, and e.g. [update_helper] [updatenotification], [git_helper] [updatenotification], [newsfeedfetcher] [newsfeed], ...

What effect does that have on 3rd-party-modules?

searching for a good module to test this. Testing with mine gave most time node_helper which is clear by construction.

Should have no negative effect, either a filename is displayed or otherwise unknown if an exception occurs.

@khassel
Copy link
Collaborator Author

khassel commented Oct 21, 2025

we could also extract the module name (from the path) for 3rd-party-modules instead of dislaying the filename which will be 95% node_helper for 3rd-party ...

@sdetweil
Copy link
Collaborator

we could also extract the module name, yes modulename filename (no path)

yes, and e.g. [update_helper] [updatenotification], [git_helper] [updatenotification], [newsfeedfetcher] [newsfeed], ...
I haven't seen any 3rd party constructed like newsfeed or calendar, node_helper calls a lib, which calls a lib, which puts out log info

in that case the module name and current file name is really all thats important.. don't need to call tree
and I'd rather have it modulename, filename, then all the modulename lines could be together after the timestamp
sort on that column in the log file

@KristjanESPERANTO
Copy link
Collaborator

Prefixes for modules would be a nice win. We'll have duplicates there too at first (because now some modules add own prefixes), but they'll gradually disappear.

@khassel
Copy link
Collaborator Author

khassel commented Oct 22, 2025

so this is my current state:

  • get last directory level before filename
  • if = js then use stripped filename, else use directory name
  • 3rd-party-mods looking good, see MMM-Remote-Control and MMM-Flights in Screenshot

We can change this logic if we find things which are ugly or not working properly.

If this is accepted I will update this PR accordingly and remove all the hardcoded duplicates introduced with the last PR (all the stuff in [...] which is not grey).

grafik

@KristjanESPERANTO
Copy link
Collaborator

Looks good to me! 🙂

@khassel
Copy link
Collaborator Author

khassel commented Oct 22, 2025

final screenshot with removed duplicates:

grafik

@khassel khassel marked this pull request as ready for review October 22, 2025 20:30
@KristjanESPERANTO KristjanESPERANTO merged commit bc0d365 into MagicMirrorOrg:develop Oct 22, 2025
12 of 14 checks passed
@khassel khassel deleted the logger branch October 22, 2025 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants