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

Settings option to dump terminal output into file. #438

Merged
merged 2 commits into from
Jul 16, 2019

Conversation

maxhora
Copy link
Collaborator

@maxhora maxhora commented Jul 8, 2019

Introduces Settings options to

  • enable/disable terminal output into log files (disabled by default)
  • option to filter out escape sequences to be added into log files (enabled by default)
  • path to directory where to create log files (default value is ApplicationData.Current.LocalCacheFolder.Path which usually equals to C:\Users\%USERNAME%\AppData\Local\Packages\53621FSApps.FluentTerminal_87x1pks76srcp\LocalCache)

Log files are created per terminal session and named to contain session creation timestamp and profile/executable name, for example, 20190708104435232_Powershell.log.

Feature requested by https://github.com/jumptrading/FluentTerminal/issues/75

@mjs
Copy link
Contributor

mjs commented Jul 9, 2019

Thanks. I've tried this out and it works quite well. Filtering of non-printable characters works but there's still plenty of "unexpected" content in there because of the way terminal updates are done. I don't know if there's much that can be done about that. Putty's "printable characters" logging does the same.

Potential future improvement: keyboard binding to enable disable recording on a per-terminal basis

Copy link
Contributor

@mjs mjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor stuff

FluentTerminal.SystemTray/Services/TerminalsManager.cs Outdated Show resolved Hide resolved
FluentTerminal.SystemTray/Services/TerminalsManager.cs Outdated Show resolved Hide resolved
@maxhora maxhora force-pushed the mr/log-ouput-to-file-upstream branch from 6506013 to f032cdf Compare July 9, 2019 13:22
@maxhora
Copy link
Collaborator Author

maxhora commented Jul 9, 2019

@felixse @mjs this, hopefully, is ready to be merged

@maxhora maxhora force-pushed the mr/log-ouput-to-file-upstream branch from f032cdf to c5f9407 Compare July 9, 2019 21:20
public void DisplayTerminalOutput(byte terminalId, byte[] output)
{
var appSettings = _settingsService.GetApplicationSettings();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still not happy with this. We should keep this as a field and update it on change (I think the UWP is already informing the SystemTray when ApplicationSettings have changed)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, there is no special Request class to inform SystemTray about changed settings or can't see any other notification mechanism. Going to add SettingsChangedRequest to be sent from UWP to Tray process.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixse here we go 49b5d4e

@maxhora maxhora force-pushed the mr/log-ouput-to-file-upstream branch from c5f9407 to 49b5d4e Compare July 12, 2019 16:43
@maxhora
Copy link
Collaborator Author

maxhora commented Jul 16, 2019

@felixse could you please check the the changes one more time, thanks!

@felixse felixse merged commit 594ebdd into felixse:master Jul 16, 2019
maxhora added a commit to maxhora/FluentTerminal that referenced this pull request Jul 19, 2019
* Settings option to dump terminal output into file.

* Notify tray process on settings change.
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