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

mutt: handle neomutt -Q var correctly #463

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

Conversation

mgedmin
Copy link
Contributor

@mgedmin mgedmin commented Nov 11, 2020

The $muttcmd -Q ... | sed ... invocation was repeated in three places, so I've extracted it into a new helper function _muttconfvar().

Closes #462.

(I looked into adding a test, but I'm not sure how. It looks like the tests are executed in Docker, but I couldn't figure out what packages are included there or how to add neomutt.)

mgedmin added a commit to mgedmin/dotfiles that referenced this pull request Nov 11, 2020
This is the same code I submitted upstream as
scop/bash-completion#463

It Works For Me (TM).
Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Packages get included in docker images automatically, the image builds reference test/test-cmd-list.txt to determine what gets pulled in, and that file is updated using test/update-test-cmd-list. If neomutt was installed as an alias to mutt and had some unit tests, that'd be enough to get it included.

Anyway the tests can be run locally without docker too, see doc/testing.txt

completions/mutt Outdated Show resolved Hide resolved
Also allow multiple spaces in case there ever are any.

Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
@mgedmin
Copy link
Contributor Author

mgedmin commented Nov 12, 2020

Packages get included in docker images automatically, the image builds reference test/test-cmd-list.txt to determine what gets pulled in,

neomutt in debian buster (10) is old enough that it still prints using the old output format.

neomutt in ubuntu 20.04 is using the new format, but it looks like your .travis.yml runs against ubuntu14. (neomutt in ubuntu 18.04 also uses the old format.)

I don't know how to check package names and versions available for centos, fedora or alpine. The name 'fedoradev' seems promising if one wants fresh new package versions.

Anyway it looks like testing this properly would need #466 to be merged as well.

mgedmin added a commit to mgedmin/bash-completion that referenced this pull request Nov 12, 2020
These tests currently fail because mutt completion is buggy.
Merging scop#463 and 465 makes all the tests pass.

This commit also includes a unit tests for the _muttconfvar helper
introduced in scop#463.
@mgedmin
Copy link
Contributor Author

mgedmin commented Nov 12, 2020

So I have some tests in #467. In my environment 'mutt' is an alias for 'neomutt', so the tests I added fail without this fix.

I could copy the _muttconfvar test from test_mutt.py in #467 into test_neomutt.py added in #466, and then the fix would be tested in CI.

(I don't see any Travis builds here, but I do get them on my fork: https://travis-ci.com/github/mgedmin/bash-completion/branches.
The failures I see there are in TestPkgConfig::test_variable and TestAvahiBrowse::test_service_types and I think I saw one failure about dselect, but I can't find it at the moment.)

mgedmin added a commit to mgedmin/bash-completion that referenced this pull request Nov 12, 2020
These tests currently fail because mutt completion is buggy.
Merging scop#463 and 465 makes all the tests pass.

This commit also includes a unit tests for the _muttconfvar helper
introduced in scop#463.
mgedmin added a commit to mgedmin/bash-completion that referenced this pull request Nov 12, 2020
These tests currently fail because mutt completion is buggy.
Merging scop#463 and 465 makes all the tests pass.

This commit also includes a unit tests for the _muttconfvar helper
introduced in scop#463.
@mgedmin mgedmin requested a review from scop January 4, 2021 14:33
@scop scop force-pushed the master branch 3 times, most recently from c0e9459 to 09307f8 Compare March 30, 2021 15:12
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.

mutt completion doesn't work with neomutt 20200626
2 participants