Skip to content

Integrate markasjunk2 features into markasjunk - marking as non-junk + learning engine #6504

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

Merged
merged 5 commits into from
Nov 10, 2018

Conversation

alecpl
Copy link
Member

@alecpl alecpl commented Oct 29, 2018

This is not finished yet. TODO:

  1. Better Non-Junk icon (at least for Elastic).
  2. Get rid of markasjunk_toolbar option. I don't like two buttons in the toolbar. One reason is not enough room there in Elastic. I propose to use only Junk button in the toolbar. When markasjunk_spam_only=false we add "mark as junk" and "mark as non-junk" into Mark menu. Also making it different on the message toolbar does not make much sense, imo.

@johndoh

@alecpl alecpl added this to the 1.4-rc milestone Oct 29, 2018
@johndoh
Copy link
Contributor

johndoh commented Oct 29, 2018

Get rid of markasjunk_toolbar option. I don't like two buttons in the toolbar. One reason is not enough room there in Elastic. I propose to use only Junk button in the toolbar. When markasjunk_spam_only=false we add "mark as junk" and "mark as non-junk" into Mark menu. Also making it different on the message toolbar does not make much sense, imo.

Does this mean that with markasjunk_spam_only=false then the icon will always be on the toolbar (similar to current markasjunk plugin) and with markasjunk_spam_only=true then the icons will always be in the mark menu?

I like having them in the mark menu because it is a mark action. People who are used to markasjunk might be confused by the change though. I agree the different settings for the different screens is a bit much.

@alecpl
Copy link
Member Author

alecpl commented Oct 30, 2018

No, I think I'm a little bit lost in all these options. What I want is two modes of operation:

  1. Default, only moving/marking as junk (what markasjunk plugin did originally): here we need only the "mark as junk" button in toolbar.
  2. The default mode plus moving/marking as non-junk: here I propose to keep the toolbar button and additionally add "mark as junk" and "mark as non-junk" into the Mark menu.

I'm not sure we even need markasjunk_spam_only option at all. Maybe we could check markasjunk_ham_mbox and/or markasjunk_ham_flag to achieve these two modes. What do you think?

@johndoh
Copy link
Contributor

johndoh commented Oct 30, 2018

I'm not sure we even need markasjunk_spam_only option at all. Maybe we could check markasjunk_ham_mbox and/or markasjunk_ham_flag to achieve these two modes. What do you think?

The markasjunk_spam_only was added recently to give markasjunk2 the same behaviour as markasjunk. Removing it would introduce a new limitation because right now the plugin could be configured with markasjunk_ham_mbox and markasjunk_ham_flag set to false so messages are not moved or flagged but a driver can still perform an action to mark the message as ham (eg the email_learn driver) which sends a copy of the message off to an email address)

@alecpl
Copy link
Member Author

alecpl commented Nov 4, 2018

I misinterpreted the behavior because of some bugs. I missed the button hiding code. I fixed bugs and made markasjunk_toolbar a boolean. Now it's ok. @johndoh could you verify?

The last part left is a better Non-Junk icon.

@johndoh
Copy link
Contributor

johndoh commented Nov 6, 2018

seems good to me. just fyi you can still get both the junk and not junk buttons on the toolbar if you do a multi folder search (that is intended in MAJ2).

@johndoh
Copy link
Contributor

johndoh commented Nov 7, 2018

this might be a little too picky but there are lots of references to "spam" (coz in the UK its commonly called that) rather than "junk". would you like me to change all function/variable names from spam -> junk? just thinking that now, with the rewrite, it would be a good time to clean up the fairly random mix of terms.

@alecpl
Copy link
Member Author

alecpl commented Nov 7, 2018

I come from a country that uses 'Spam' so I don't have a strong opinion. Besides spam/ham looks better than junk/nonjunk in some places, imo. Also, there is Spamassassin, not Junkassassin ;)

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.

2 participants