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

Adds a debug option for espanso when executing shell commands for greater context. #359

Conversation

deckarep
Copy link
Contributor

In reference to the following: #355

I've added a debug flag for shell commands to utilize.

  • When not provided debug defaults to false.
  • When non-parseable debug defaults to false.
  • When provided, error log is thrown which captures: cmd string, exit_status, stderr, stdout in a single log-line.
  • I can attempt to add a relevant unit-test if this looks like a reasonable patch.

@federico-terzi federico-terzi changed the base branch from master to dev July 10, 2020 06:29
Copy link
Collaborator

@federico-terzi federico-terzi left a comment

Choose a reason for hiding this comment

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

Hey,
Thank you for your contribution! I left you only a minor comment and then we are ready to go :)
Keep in mind that all changes are first merged into the dev branch and then, when releasing a new version, they get merged into master.

Cheers :)

src/extension/shell.rs Outdated Show resolved Hide resolved
@federico-terzi
Copy link
Collaborator

Hey @deckarep,
I'm currently away and I'll get back home by Monday, I'll make sure to review the changes and merge it into dev next week.
Thanks again for your help :)

@deckarep
Copy link
Contributor Author

Sounds good @federico-terzi!

One question would you also be interested in PRs that provide optimizations both big and small? Some examples that come to mind I see are PRs where owned strings are allocated instead of using references. Of course bigger optimizations could be present in hot paths that might be more impactful.

@federico-terzi
Copy link
Collaborator

One question would you also be interested in PRs that provide optimizations both big and small? Some examples that come to mind I see are PRs where owned strings are allocated instead of using references. Of course, bigger optimizations could be present in hot paths that might be more impactful.

@deckarep In general, I'd like to discuss all the PRs that get proposed as I really want to avoid wasting people time :)

I originally started espanso to learn Rust, so some of the early work was definitely not idiomatic. If I had the time, I would completely re-write it from scratch now that I have much better knowledge, but that would be mostly for aesthetics as espanso is already fast enough for most use-cases.

@federico-terzi federico-terzi merged commit 44322a7 into espanso:dev Jul 14, 2020
@federico-terzi
Copy link
Collaborator

Merged, thanks for the help :)

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.

None yet

2 participants