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

Change /clear behaviour. Closes issue #855. #874

Merged
merged 2 commits into from
Oct 5, 2019
Merged

Change /clear behaviour. Closes issue #855. #874

merged 2 commits into from
Oct 5, 2019

Conversation

spiridoncha
Copy link
Contributor

I just change /clear behavior, but it's not configurable. Should I implement some option for /clear, which change behavior or something like that?

@boothj5
Copy link
Collaborator

boothj5 commented Nov 6, 2016

@spiridoncha apologies for the delay in responding, I took a bit of a break after the last release.

I think making it configurable would be a good idea. If you'd like to discuss let me know here.

Thanks,

James,

@spiridoncha
Copy link
Contributor Author

spiridoncha commented Nov 6, 2016

@boothj5 thank you for your response.
How should I make this behavior configurable?
I see some cases:

  1. Create option for profrc in section [ui] with name clear.persist_history and default value is true or something like that.
  2. Create alternative /clear command with new behavior and other name.
  3. Create argument for current version of /clear with syntax /clear full or similar.

@boothj5
Copy link
Collaborator

boothj5 commented Nov 6, 2016

I think option 1 would be best, but maybe defaulting to false.

Usually when I allow changing existing behaviour with a configuration option I'll default to the original behaviour (clearing history in this case), just so current users get the same behaviour as before.

@boothj5
Copy link
Collaborator

boothj5 commented Nov 7, 2016

A couple of comments:

  1. Most config options can be set with commands, in this case perhaps:
    /clear set persist_history on - sets clear.persist_history=true
    /clear set persist_history off - sets clear.persist_history=false
    If you have any other ideas, let me know.

  2. When the window buffer is full (1000 lines) the /clear command with clear.persist_history=true doesn't clear the screen. I came across the same issue when I briefly looked into clearing the screen, it might require some changes to how the screen is buffered.

@jubalh
Copy link
Member

jubalh commented Feb 20, 2019

@spiridoncha still interested in working on this?

@jubalh
Copy link
Member

jubalh commented Oct 5, 2019

I'll merge this and work on it in separate commits.

@jubalh jubalh added this to the 0.8.0 milestone Oct 5, 2019
@jubalh jubalh merged commit d7c0036 into profanity-im:master Oct 5, 2019
@jubalh
Copy link
Member

jubalh commented Oct 5, 2019

@spiridoncha thanks for your contribution!

jubalh added a commit that referenced this pull request Oct 5, 2019
Regards #855

#874 brought us the
`/clear` command. The author of that patch couldn't follow up with the
review boothj5 did.

So the autocompletion and updated help was missing.
This commit adds it.
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.

3 participants