Bug 1646 - sa-learn "--single" option should be "--stdin" or similar
Summary: sa-learn "--single" option should be "--stdin" or similar
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Learner (show other bugs)
Version: 2.50
Hardware: Other other
: P5 normal
Target Milestone: 2.60
Assignee: Theo Van Dinter
URL:
Whiteboard:
Keywords: backport
Depends on:
Blocks:
 
Reported: 2003-03-15 12:08 UTC by Daniel Quinlan
Modified: 2003-06-18 22:06 UTC (History)
1 user (show)


Attachment Type Modified Status Actions Submitter/CLA Status
Patch to implement my proposal patch None Brett A. Thomas [NoCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Quinlan 2003-03-15 12:08:59 UTC 
This option name is confusing.

It strikes me that it would be better to change --file to --single
and add a --stdin option that redirects *either* --mbox or --single
to be on stdin instead of a file on the command line.

--file can remain an alias for --single or we can deprecate it (deprecation
might be better).

I'd like to get this fixed in 2.51 since the --single option is new since
2.50.
Comment 1 Justin Mason 2003-03-15 13:01:11 UTC 
Rather than having --stdin, let's just support "-" in std UNIX style.
But I think --single is worth keeping for backwards compat, even
though it's so new, since we
suggest using it in .muttrc's etc.
Comment 2 Brett A. Thomas 2003-03-15 21:58:36 UTC 
Maybe I'm mising something obvious here, but why does it need to be an option,
at all?  If I do "sa-learn --spam", it simply says "Learned from 0 messages." 
What's worse, if I do "sa-learn --spam < foo", it still says "Learned from 0
messages," even though it might not be unreasonable to assume that what just
happened is that sa-learn had already seen the message in question.

Thinking about this a lot more, here's what makes a lot of sense to me, because
Dan is right that there's basically not any way to do "sa-learn --spam --mbox <
foo":

Make sa-learn's basic invocation look like:

sa-learn [options] [file1..fileN]

--mbox becomes an option, stating that the data is in mbox format.  If not
specified, the input source is presumed to be seperate RFC822 messages.
--file becomes ignored for backwards compatability
--single becomes ignored for backwards compatability
--directory becomes ignored for backwards compatability

If no files are specified, sa-learn awaits input on stdin.
If one or more files are specified, sa-learn reads them as individual messages
(or mboxes if --mbox is present).
If one or more directories are specified, sa-learn reads them as directories.

Conceivably, this means you could say "sa-learn --spam --mbox spam/" to learn a
directory of mboxes.

This proposal would cause sa-learn to work much more like other Unix programs,
and be more intuitive to most users.

If folks think this is a good idea, I'd be happy to try out a patch, but it
sounds substantial enough that if folks think this is a bad idea (or want to do
it themselves ;) I'll hold off.  I'd also presume that a patch this big would go
against Dan's desire to get this done for 2.51.
Comment 3 Brett A. Thomas 2003-03-21 18:02:35 UTC 
Created attachment 798 [details]
Patch to implement my proposal
Comment 4 Brett A. Thomas 2003-03-21 18:02:56 UTC 
I realize this was never actually accepted beyond "NEW" but it seemed like an
easy way to get my feet wet with the source.  Dunno if everyone thinks my
proposal is worthless, but here's a patch that implements it in case anyone
likes it.
Comment 5 Daniel Quinlan 2003-05-25 17:23:52 UTC 
I'll look at this patch.
Comment 6 Antony Mawer 2003-05-25 17:37:09 UTC 
Subject: Re: [SAdev]  sa-learn "--single" option should be "--stdin" or similar 


BTW, I never commented on this, sorry. ;)
FWIW, it looks like a good UI to me...
Comment 7 Duncan Findlay 2003-06-06 23:36:38 UTC 
Dan, still loking at it? Looks good to me, FWIW.
Comment 8 Theo Van Dinter 2003-06-15 09:49:47 UTC 
it looks good to me too.  quinlan, any updates?
Comment 9 Theo Van Dinter 2003-06-16 11:41:56 UTC 
since 3 devs have said ok, I'm going to apply this to 2.60.
Comment 10 Theo Van Dinter 2003-06-16 11:46:45 UTC 
committed.
Comment 11 Michael Bell 2003-06-19 01:21:27 UTC 
Small enhancement suggestion - debug output should clarify that use_bayes is 
off. Why?

I ran sa-learn as a test of 2.6CVS, and got that it was LEARNING...(listed 500 
lines, without tokens). It then said learned from 0 messages.

This was a config mistake on my part - left use_bayes 0 in local.cf ... but it 
wouldn't hurt to SAY so in the -D output!
Comment 12 Theo Van Dinter 2003-06-19 06:06:25 UTC 
the suggestion is not related to this bug.  please open a new bug with the request.