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

git add . add package common-phrases #151

Merged
merged 3 commits into from
Jan 12, 2025
Merged

Conversation

KarimAL-Shamy
Copy link
Contributor

No description provided.

Copy link
Collaborator

@smeech smeech left a comment

Choose a reason for hiding this comment

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

A nice package of straightforward trigger/replace pairs with no script, shell or potentially malicious code.

The installation instructions in README.md need correcting or removing, and we'll be good to merge.

Thank you.

## Installation

```bash
espanso install commonPhrases
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Hub automatically shows the installation instructions for all the packages, so it isn't necessary to include them here.

If you want to, however, the line will be espanso install common-phrases to match the package name, and espanso restart isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ive fixed that 👌 thank you for your comment

ps. ive used one of the packages (quick-translate) and had issue with the rtl languages so i tired multiple solutions until one worked decent still some little issues, so i am wondering should i just add my changes or should i get in contact with someone?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for doing that. The package had passed its automated checks so I'm about to merge it.

As far as quick-translate is concerned, ideally you would fork @p0ly60n's repo https://github.com/p0ly60n/quick-translate or https://github.com/p0ly60n/hub-quick-translate and submit a PR with your improvements, for them to PR to espanso/hub, attributing you accordingly. Although @p0ly60n isn't very active on GitHub, the package is relatively recent, and he/she did communicate via the Espanso Discord server. If you get no response after a month or so, by all means fork the Hub and PR directly - this is what I have had to do with some of the older PRs which needed fixing. Let me know if you need any help.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello,
just got the notification. How can I help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay thanks @smeech.
hi @p0ly60n, i will be submitting a PR there and we can chat about it there.
ps. i loved the quick-translate package

@smeech smeech merged commit f170b11 into espanso:main Jan 12, 2025
1 check passed
@smeech
Copy link
Collaborator

smeech commented Jan 12, 2025

All done: https://hub.espanso.org/common-phrases.

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