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

lineoperations: remove every n-th line #860

Merged
merged 1 commit into from
Jul 12, 2019

Conversation

lpaulsen93
Copy link
Contributor

Imlements a new feature which allows to remove every n-th line. The user can enter the value of 'n' in a dialog. Implements #772.

@lpaulsen93 lpaulsen93 mentioned this pull request May 13, 2019
@lpaulsen93 lpaulsen93 force-pushed the lineops-rm-every-n branch from 3d50545 to 3783e8c Compare May 13, 2019 20:34
@lpaulsen93
Copy link
Contributor Author

Don't merge yet! There is an error: need to add lineoperations to POTFILES.in. Will do that tomorrow.

@elextr elextr changed the title lineoperations: remove every n-th line [WIP] lineoperations: remove every n-th line May 13, 2019
Imlements a new feature which allows to remove every n-th line.
The user can enter the value of 'n' in a dialog. Implements geany#772.
@lpaulsen93 lpaulsen93 force-pushed the lineops-rm-every-n branch from 3783e8c to 2833d96 Compare May 14, 2019 16:42
@lpaulsen93
Copy link
Contributor Author

Done. Updated POTFILES.in and also completed the README.

@lpaulsen93 lpaulsen93 changed the title [WIP] lineoperations: remove every n-th line lineoperations: remove every n-th line May 14, 2019
@frlan
Copy link
Member

frlan commented May 18, 2019

@smostertdev should take over review maybe

@lpaulsen93
Copy link
Contributor Author

@frlan: we have nearly waited 2 month for a reply. I vote for merging this. It's quite a simple change and my tests looked good. Of course you can test it yourself if you like.

@elextr
Copy link
Member

elextr commented Jul 12, 2019

@smostertdev is the lineoperations maintainer I think.

@lpaulsen93
Copy link
Contributor Author

@elextr: yeah, but he didn't reply so I contacted frlan as he seems to be kind of the manager of geany-plugins.

@frlan
Copy link
Member

frlan commented Jul 12, 2019

I cannot do anything about how people responses. However, I did not found anything cruzial while reading over code. Let's test it in practice.

@frlan frlan merged commit 0cd74f5 into geany:master Jul 12, 2019
@lpaulsen93
Copy link
Contributor Author

@frlan: of course you can't. Please don't get me wrong: I did not mean to criticise you or smostertdev. It's just that it is a simple change and I think it's better to merge it at some point than to keep it open for several months. If it was a larger PR - yeah then we definitely would need to wait.

@elextr
Copy link
Member

elextr commented Jul 12, 2019

@LarsGit223 my comment was actually just meant to be a "subtle"??? re-ping :)

@lpaulsen93 lpaulsen93 deleted the lineops-rm-every-n branch July 15, 2019 18:44
@smostertdev
Copy link
Contributor

Glad to see that everyone is more on top of things than me. I just double checked it, compiled it and tested it and it looks great. Don't see any issues with to code, well written @LarsGit223

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants