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

add ability to reset matches but keep regions #145

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vyacheslavs
Copy link

after [reset] command i need always remove regions by [dregion] command again and again.
this patch adds option to [reset] command to keep current region list

@sriemer
Copy link
Member

sriemer commented Dec 1, 2015

The idea is good. But the code style is bad once again.
If you look at handlers.h, then you should notice that options added to commands always use the underscore '_' to split words. Examples: 'scan_data_type', 'region_scan_level',... So 'reset keep-regions' makes that inconsistent. It should be 'reset keep_regions' instead.

All other options of that kind are implemented as parameters of the 'option' command. It should be discussed if something like 'option reset_keeps_regions 1' would be better to set this. Maybe @coolwanglu has a preference.

@coolwanglu
Copy link
Contributor

How about

reset [all]
reset matches
reset regions

@sriemer
Copy link
Member

sriemer commented Dec 28, 2015

Resetting regions without the matches might cause unpredicted behaviour. I would leave that out for now. The rest sounds very good.

@sriemer sriemer added this to the v0.17 milestone Jun 10, 2016
@12345ieee 12345ieee mentioned this pull request Oct 5, 2017
8 tasks
@12345ieee
Copy link
Member

I agree that in the current state this isn't merge-able, but the idea of chaining a dregion <correct list> to any reset is reasonable.

I'm removing it from milestones, until interest picks up for this feature and it can be implemented better.

@12345ieee 12345ieee removed this from the v0.17 milestone Oct 9, 2017
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

4 participants