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

editor: move completion functions #4025

Merged
merged 9 commits into from
Sep 7, 2023
Merged

editor: move completion functions #4025

merged 9 commits into from
Sep 7, 2023

Conversation

flatcap
Copy link
Member

@flatcap flatcap commented Sep 6, 2023

NOTE: this is working, but I need to do some more testing.

Deep within libenter there are lots of dependencies caused by auto-completion and history.

Users call mw_get_field() with arguments like COMP_ALIAS which determine which completion function to use and where to save the history.
Worse still is the fact that the alias code is in libenter.
and browser code and notmuch code and pattern code...

This PR creates a basic Completion API that passes in a function to do the work.
It should help with: #3955 Neomutt completion API

All the functions on the right are now in more appropriate places.
The abstraction isn't perfect, but it's a step forward.

Source: graphviz, svg

@flatcap flatcap added the topic:refactoring Code refactoring label Sep 6, 2023
@flatcap flatcap self-assigned this Sep 6, 2023
@Amudtogal
Copy link
Contributor

Hey flatcap,
this is amazing, thank you for the great refactor.
I will rebase my autocompletion branch on this, and this should speed things up.

Copy link
Contributor

@Amudtogal Amudtogal left a comment

Choose a reason for hiding this comment

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

Excellent, this is now so much clearer (at least in my head).

It's great that each user (alias, browser etc) manages their own completion function, without needing too much logic.

Copy link
Contributor

@Amudtogal Amudtogal left a comment

Choose a reason for hiding this comment

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

Excellent, this is now so much clearer (at least in my head).

It's great that each user (alias, browser etc) manages their own completion function, without needing too much logic.

Copy link
Contributor

@Amudtogal Amudtogal left a comment

Choose a reason for hiding this comment

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

Excellent, this is now so much clearer (at least in my head).

It's great that each user (alias, browser etc) manages their own completion function, without needing too much logic.

@Amudtogal
Copy link
Contributor

just one thing I spotted when compiling:

browser/complete.c:122:20: warning: implicit declaration of function ‘mutt_mb_is_shell_char’; did you mean ‘mutt_mb_is_pathname_char’? [-Wimplicit-function-declaration]
  122 |        (i > 0) && !mutt_mb_is_shell_char(wdata->state->wbuf[i - 1]); i--)
      |                    ^~~~~~~~~~~~~~~~~~~~~
      |                    mutt_mb_is_pathname_char

how would i best submit tiny patches like this?
is it best in a pull request, or assemble them along with the completion api branch?

@flatcap
Copy link
Member Author

flatcap commented Sep 6, 2023

browser/complete.c:122:20: warning: implicit declaration of function ‘mutt_mb_is_shell_char’; did you mean ‘mutt_mb_is_pathname_char’? [-Wimplicit-function-declaration]

Hmm... I don't understand.

neomutt/browser/complete.c

Lines 121 to 124 in b99bce7

for (i = wdata->state->curpos;
(i > 0) && !mutt_mb_is_shell_char(wdata->state->wbuf[i - 1]); i--)
{
}

should be fine, because the file includes mutt/lib.h

#include "mutt/lib.h"

which includes mbyte.h

#include "mbyte.h"

which declares mutt_mb_is_shell_char()

bool mutt_mb_is_shell_char(wchar_t ch);

I can't repeat the error myself with gcc or clang.

is it best in a pull request, or assemble them along with the completion api branch?

if you can figure out what's happened, push a commit to this branch.

how would i best submit tiny patches like this?

Generally, feel free to create/add to a branch [trivial] in the neomutt repo.
I'll merge it into my next bundle of tidying.

@Amudtogal
Copy link
Contributor

Fair enough, it is gone on my side as well, after make clean && make.
Maybe it was a lingering object file...
If it comes back up, I will contribute to [trivial]

@Amudtogal
Copy link
Contributor

Ah found it:
further down in my dev branch, I am renaming mutt_mb_is_shell_char to mutt_mb_is_pathname_char (more verbose, and it was only used in one spot anyway).
This was causing the compilation problem.

I have rebased my dev branch on this pull request to get started.

Base automatically changed from devel/timeout2 to main September 7, 2023 13:25
@flatcap flatcap marked this pull request as ready for review September 7, 2023 13:26
@flatcap flatcap requested a review from a team as a code owner September 7, 2023 13:26
@flatcap flatcap merged commit 4f2e4b8 into main Sep 7, 2023
12 checks passed
@flatcap flatcap deleted the devel/enter branch September 7, 2023 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:refactoring Code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants