Frezyx / talker

☎️ Advanced error handler and logger for dart and flutter apps
https://pub.dev/packages/talker
MIT License
478 stars 57 forks source link

Custom timestamp #232

Closed abdelaziz-mahdy closed 2 months ago

abdelaziz-mahdy commented 3 months ago

initial work to allow this change https://github.com/Frezyx/talker/issues/231

abdelaziz-mahdy commented 3 months ago

if the custom time function should be the way to go let me know, in that case i think it will be more customizable but keeping both options is better to have an easy choice and a fully custom one

Frezyx commented 3 months ago

Hello @abdelaziz-mahdy ! Sorry for the delay 🙏🏻 This is a very important feature for the library, but I have suggestions for how it could be implemented. Can we provide TimeFormat or formatting method as generateMessage method named (optional) argument ?

Example of code below:

  String generateTextMessage({TimeFormat timeFormat = TimeFormat.timeAndSeconds}) {
    return '$<formatted time by method>$displayTitle$message$displayStackTrace';
  }

This implementation resolve some issues:

1) Providing TimeFormat for all TalkerData instances is not very clear logic. 2) We can change the log format during the application's operation, but it won't be applied to all created records in history. 3) TalkerHistory saving in runtime in application memory. This field adding more unused data for heap.

In this case (providing timeFormat as method arg) we can setup it in TalkerSettings in package initialization one time. What you think about that ?

abdelaziz-mahdy commented 3 months ago

I like that idea, since changing all of the methods was alot of work and didn't think it will be a clean approach, will check it and let you know

abdelaziz-mahdy commented 3 months ago

my only problem is that i dont know how to pass it to settings and use it, right now my changes allow to use any time format since i added the param to the TalkerData but the point of passing the time format to it is the main problem for me

abdelaziz-mahdy commented 3 months ago

using your idea i think we will have a problem with this

extension TalkerDataInterfaceListExt on List<TalkerData> {
  /// The method allows you to get
  /// full text of logs or history
  String get text {
    final sb = StringBuffer();
    for (final data in this) {
      sb.write('${data.generateTextMessage()}\n');
    }
    return sb.toString();
  }
}

where my previous approach didnt have a problem with it

well, i fixed it ignore this.

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 98.33%. Comparing base (39b5aee) to head (b891a1c). Report is 1 commits behind head on master.

Files Patch % Lines
packages/talker/lib/src/utils/time_formatter.dart 45.45% 6 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #232 +/- ## ========================================== - Coverage 99.82% 98.33% -1.49% ========================================== Files 28 17 -11 Lines 573 421 -152 ========================================== - Hits 572 414 -158 - Misses 1 7 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

abdelaziz-mahdy commented 3 months ago

i think its now ready for a review, let me know what should be added or changed, or if its all good

Frezyx commented 2 months ago

Hello @abdelaziz-mahdy ! Thank you for contribution ❤️
This functionality now available in 4.3.0 package version