chromelyapps / Chromely

Build Cross Platform HTML Desktop Apps on .NET using native GUI, HTML5, JavaScript, CSS, Owin, AspNetCore (MVC, RazorPages, Blazor)
MIT License
2.98k stars 279 forks source link

Configured CefLogFile path overridden to include date #317

Closed MichelJansson closed 3 years ago

MichelJansson commented 3 years ago

The CefLogFile as configured in "CustomSettings" is overridden to include date, with no way to prevent that behavior. It makes sense to have the filename be autogenerated for the default name (which it does), but not when explicitly specified.

The below config will actually write to the file logs\my.cef_20210409.log. (yyyyMMdd)

config.CustomSettings = new Dictionary<string, string>()
{
    ["cefLogFile"] = "logs\\my.cef.log",
};

Suggested solution

This should be changed to just take the unchanged value of the setting: https://github.com/chromelyapps/Chromely/blob/1f95b7d1475cd56bd9a50b1d8df7140e42810e22/src/Chromely/Browser/CefSettingsExtension.cs#L118

The default name is handled here, and should be unaffected by this change: https://github.com/chromelyapps/Chromely/blob/1f95b7d1475cd56bd9a50b1d8df7140e42810e22/src/Chromely/WindowController.Run.cs#L68


Thank you for your work on this project!

mattkol commented 3 years ago

@MichelJansson

So if I understand the suggestion

On line: https://github.com/chromelyapps/Chromely/blob/1f95b7d1475cd56bd9a50b1d8df7140e42810e22/src/Chromely/Browser/CefSettingsExtension.cs#L118

we should have:

 cefSettings.LogFile = config.CustomSettings["cefLogFile"]; 

Right?

Thank you for your work on this project!

yw

MichelJansson commented 3 years ago

@mattkol Thanks for looking into this.

I would say that since this is all within a foreach, line 118 would read: cefSettings.LogFile = setting.Value and of course, line 116 and 117 is then not needed.

mattkol commented 3 years ago

@MichelJansson thanks for pointing this out, I could not remember why it was done this way in first place.

MichelJansson commented 3 years ago

Good stuff @mattkol - Thanks!