fixed regexp to get query command from mutt and added support for multi-line address fields #2

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

marklee77 commented Dec 23, 2012

title says it all.

nice plugin.

Owner

caio commented Dec 25, 2012

Hi Mark,

Thanks for your contribution. Could you please split your patch in two? The query_command regexp improvement I can merge right away, but the multi-line support has two problems: (1) it's a bit too greedy IMO, if going down this route I'd like to have this as a (off by default) setting and (2) it breaks the support for custom patterns.

Cheers and happy holidays!

Contributor

marklee77 commented Dec 25, 2012

On 12-12-25, Caio Romão wrote:

Thanks for your contribution. Could you please split your patch in
two? The query_command regexp improvement I can merge right away, but

Possibly. I'm not sure how to do that in a straightforward way with git
though I suppose I could just role back to the original and then do each
of the changes as a separate commit.

The first change is just a pretty small correction to the regex for
figuring out the command from mutt -Q:

'^query_command="(.)\s%s."\n'

Now, this just copies the command up to '%s', and assumes that that will
be the last useful thing on the line. It should be fairly simple to just
get the whole like and keep the %s. Then, when the command is executed,
check if there is %s in the command, and if there is replace it with the
keyword, otherwise just append the keyword to the command as is done
currently.

I might commit that change as a new patch if it seems reasonable.

the multi-line support has two problems: (1) it's a bit too greedy
IMO, if going down this route I'd like to have this as a (off by
default) setting and (2) it breaks the support for custom patterns.

Well, my understanding is that the email message should be a bunch of
header fields, each starting with "header-name:", then a blank line,
then the message body, so the test should just back up to the last line
with either a : present (indicating a header name) or a blank line, then
compares that line against the qcc_pattern--so users are still able to
specify custom fields.

The only real bug with it that I can see is that if a user starts a line
with something like 'From:' in the body of the message then the script
won't realize we aren't in the header and will do auto completion, but
that seems unlikely to come up unless the user is deliberately inducing
the bug.

Mark Stillwell
Lecturer, Engineering Computing
Building 52, Cranfield University, Cranfield, Beds MK43 0AL
W: www.cranfield.ac.uk E: m.stillwell@cranfield.ac.uk

Owner

caio commented Dec 25, 2012

On Tue, Dec 25, 2012 at 9:10 PM, Mark Stillwell
notifications@github.com wrote:

On 12-12-25, Caio Romão wrote:

Thanks for your contribution. Could you please split your patch in
two? The query_command regexp improvement I can merge right away, but

Possibly. I'm not sure how to do that in a straightforward way with git
though I suppose I could just role back to the original and then do each
of the changes as a separate commit.

Yup, rolling back seems easier - especially if you decide to go down
the route you describe below. Though if you're interested it learning
a bit deeper about git: this could be easily accomplished by using the
-p switch on for the checkout command (git checkout -p HEAD^).

The first change is just a pretty small correction to the regex for
figuring out the command from mutt -Q:

'^query_command="(.)\s%s."\n'

Now, this just copies the command up to '%s', and assumes that that will
be the last useful thing on the line. It should be fairly simple to just
get the whole like and keep the %s. Then, when the command is executed,
check if there is %s in the command, and if there is replace it with the
keyword, otherwise just append the keyword to the command as is done
currently.

I might commit that change as a new patch if it seems reasonable.

Absolutely! Sounds like a way better approach than assuming a specific
format. I honestly don't use this feature as I'd rather configure
things explicitly, but it does make a lot of sense for a drop-in use
case.

the multi-line support has two problems: (1) it's a bit too greedy
IMO, if going down this route I'd like to have this as a (off by
default) setting and (2) it breaks the support for custom patterns.

Well, my understanding is that the email message should be a bunch of
header fields, each starting with "header-name:", then a blank line,
then the message body, so the test should just back up to the last line
with either a : present (indicating a header name) or a blank line, then
compares that line against the qcc_pattern--so users are still able to
specify custom fields.

The only real bug with it that I can see is that if a user starts a line
with something like 'From:' in the body of the message then the script
won't realize we aren't in the header and will do auto completion, but
that seems unlikely to come up unless the user is deliberately inducing
the bug.

You're right about email messages, but bear in mind that the plugin's
objective is to be useful outside of the mutt/mail scope - that's why
I'd like this greediness off by default. It could definitely be
enabled in the case where there's no configuration done by the user
though 1

Contributor

marklee77 commented Dec 25, 2012

Yup, rolling back seems easier - especially if you decide to go down
the route you describe below. Though if you're interested it learning
a bit deeper about git: this could be easily accomplished by using the
-p switch on for the checkout command (git checkout -p HEAD^).

Right, okay, I did two new commits, the first fixes the query command to
fix getting it from mutt and support %s, and undoes multi-line headers.
The second restores multi-line, which works well for my usage case, but
could probably use some refinement.

Mark Stillwell

Owner

caio commented Dec 25, 2012

Great!

I've squashed-merged the first two commits into 8d34498 (in the from_marklee77 branch). I'll take a look at 070e500 during the weekend and follow up with a release and update here.

Thanks again!

@caio caio added a commit that referenced this pull request Dec 29, 2012

@caio caio Merge branch 'from_marklee77'
Closes pull request #2:
    #2

* from_marklee77:
  Make multiline support configurable
  restored multiline support
  Add support for "%s" in qcc_query_command
987a67a
Owner

caio commented Dec 29, 2012

All done and v0.3 released. Thanks again for your contribution!

caio closed this Dec 29, 2012

Contributor

marklee77 commented Dec 29, 2012

On 12-12-29, Caio Romão wrote:

All done and v0.3 released. Thanks again for your contribution!

Glad I could help!

Dr Mark Stillwell
Lecturer, Engineering Computing
Building 52, Cranfield University, Cranfield, Beds MK43 0AL
W: www.cranfield.ac.uk E: m.stillwell@cranfield.ac.uk

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