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.

Sign up for GitHub

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

Jump to bottom

Add history flushes on each command to the powerline themes #1883

Merged
merged 3 commits into from
Oct 17, 2021
Merged

Add history flushes on each command to the powerline themes #1883

merged 3 commits into from
Oct 17, 2021

Conversation

ira-bv
Copy link
Contributor

@ira-bv ira-bv commented May 19, 2021

Description

History autosave enabled for a few more themes I use.

Motivation and Context

Was only supported by one theme, I wanted it on the powerline-multiline too.

How Has This Been Tested?

I eat my own dogfood :-)

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.

davidpfarrell
davidpfarrell suggested changes May 19, 2021
View reviewed changes
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Greetings! Always great to see ppl improving the Powerline themes :)

I'll look into this a bit more later, but I wanted to leave a comment that jumped to mind during a quick view of the PR.

themes/powerline-plain/powerline-plain.base.bash Outdated Show resolved Hide resolved
@ira-bv ira-bv force-pushed the ira/autosave-history-plml branch from e4a5948 to 10713d5 Compare May 19, 2021 16:19
davidpfarrell
davidpfarrell suggested changes May 20, 2021
View reviewed changes
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

OK I had a chance to further-review the changes and have some final feedback.

Lemme know if you have any questions.

plugins/available/thefuck.plugin.bash Outdated Show resolved Hide resolved
themes/powerline/powerline.base.bash Show resolved Hide resolved
themes/powerline-naked/powerline-naked.base.bash Outdated Show resolved Hide resolved
themes/powerline-multiline/powerline-multiline.base.bash Outdated Show resolved Hide resolved
@NoahGorny
Copy link
Member

@ira-bv this is a great PR- if you could address the comments from @davidpfarrell I will surely merge this sooner then later !

let me know what you think 😄

@davidpfarrell
Copy link
Contributor

Hey @NoahGorny , I know we want to give @ira-bv some time to reply/work on this, but I wanted to put this out there now while I'm thinking of it:

Please don't close this PR due to inactivity - If @ira-bv can't make it back to address the changes, I'll pick it up and take it over the line. My requested changes are pretty light and I think the PR is valuable to my beloved powerline themes :)

Thanks!

@ira-bv
Copy link
Contributor Author

ira-bv commented Aug 8, 2021 •
edited
Loading

Sorry, I was AFK for a week. I'll try to split this up and clean up this week.

@NoahGorny
Copy link
Member

gentle ping @ira-bv 😄

@ira-bv ira-bv force-pushed the ira/autosave-history-plml branch from 3d2c3bb to 9f4c71d Compare October 10, 2021 12:24
@ira-bv
Copy link
Contributor Author

ira-bv commented Oct 10, 2021

breaking up the PR

@ira-bv ira-bv requested a review from davidpfarrell October 10, 2021 12:28
davidpfarrell
davidpfarrell approved these changes Oct 10, 2021
View reviewed changes
Copy link
Contributor

@davidpfarrell davidpfarrell left a comment

Choose a reason for hiding this comment

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

Thanks for following up !

@NoahGorny
Copy link
Member

@ira-bv, please rebase on the latest master in order to fix the broken ci

@seefood
Merge branch 'master' of https://github.com/bash-it/bash-it into ira/…
d6239cf 
…autosave-history-plml

* 'master' of https://github.com/bash-it/bash-it: (114 commits)
  ci: Bump go to 1.17 from 1.14
  skip go tests when go is not available
  plugins: Add ble.sh plugin
  clean up pyenv plugin
  Lint: prepare `lib/utilities` for `shellcheck` (Bash-it#1933)
  plugin/base: improvements (Bash-it#1930)
  plugins/percol: `bind`
  completion/git: `shfmt` && `shellcheck`
  completion/git: expand search range
  plugin/percol: `shellcheck` & `shfmt`
  plugins/percol: use `_command_exists`
  completion/pip: simplify code flow
  plugin/less-pretty-cat: remove `|| cat`
  completion/wpscan: simplify code flow (whitespace)
  plugins/less-pretty-cat: simplify code flow
  plugins/less-pretty-cat: use `_command_exists`
  lib/helpers: cite `_bash-it-find-in-ancestor()`
  gradle: adopt `_bash_it_find_in_ancestor()`
  lib/helpers: new function `_bash-it-find-in-ancestor()`
  completion/laravel: simplify code flow
  ...
@ira-bv
Copy link
Contributor Author

ira-bv commented Oct 11, 2021

May be superseded by #1951?

@gaelicWizard
Copy link
Contributor

May be superseded by #1951?

I think it would be better to merge this one (#1883) than to wait for #1951. I expect there'll be a lot of discussion about how "automatic" it should be, and there's at least one idea that I really like from @cornfeedhobo that I haven't even implemented yet.

@ira-bv
Copy link
Contributor Author

ira-bv commented Oct 14, 2021

well, merge away...

@NoahGorny NoahGorny merged commit 7987e4b into Bash-it:master Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidpfarrell davidpfarrell davidpfarrell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ira-bv @NoahGorny @davidpfarrell @gaelicWizard @seefood