Re: [Nano-devel] New Feature: completion from external dictionary

[Benno Schulenberg]

Op 09-07-18 om 21:45 schreef Marco Diego Aurélio Mesquita:
> +             while (sint->dict != NULL) {
> +                     struct filestruct *item = sint->dict;
> +                     sint->dict = sint->dict->next;
> +                     free(item);
> +             }

You could have dropped this.  It is not needed to make things work.

> +struct filestruct;
> +     /* Foreward declaration of filestruct. */

Okay for now, but better send a separate patch first to reorder the
declarations.

> +     char *completer;
> +             /* The path to the completer dicitonary to this type of file. */
> +     struct filestruct *dict;
> +             /* The dicitonary cache to this type of file. */

This is confusing.  'completer' is the path to a dictionary, so I would
call that 'dictionary'.  And 'dict' is this dictionary read into memory,
providing a long list of possible completions, so I would call it...
'completions', or maybe 'list_of_words'.

Also, putting them together is confusing.  The things at the start of
the syntaxtype are things that the user specified explicitly.  At the
end of the syntaxtype follow two things that nano has determined.  So,
put this 'list_of_words' after 'nmultis' and before '*next'.

> +bool is_good_file(char *file);

If you put the reading of the dictionary into files.c, you don't
need this declaration.

>       live_syntax->headers = NULL;
>       live_syntax->magics = NULL;
>       live_syntax->linter = NULL;
> +     live_syntax->completer = NULL;
> +     live_syntax->dict = NULL;
>       live_syntax->formatter = NULL;

Again, separate what is explicitly specified from what comes about
as a result.

> +filestruct *get_dict(char *shard)
> +{
> +     char *file = openfile->syntax->completer;

Superfluous alias.

> +     filestruct *lines = openfile->syntax->dict, *tmpline = NULL, *curline = 
> NULL;

Too many declarations on one line.  Split into two.

> +     if (lines)
> +             goto end;
> +
> +     statusbar(_("Loading dictionary, please wait"));

Don't print this message before you're actually going to read a file.

> +     /* Don't open directories, character files, or block files. */
> +     if (!is_good_file(file))
> +             goto end;
> +
> +     /* Open the included syntax file. */
> +     rcstream = fopen(file, "rb");

Move the reading of the file to files.c.

> +     char *buf = NULL;
> +     ssize_t len;
> +     size_t n = 0;
> +     size_t lineno = 0;

Then these declarations won't sit somewhere in the middle but
at the start of a function, as they should.

> +     while ((len = getline(&buf, &n, rcstream)) > 0) {
> +             tmpline = curline;
> +             curline = nmalloc(sizeof(filestruct));
> +             curline->next = NULL;
> +             curline->prev = tmpline;
> +             if (curline->prev != NULL)
> +                     curline->prev->next = curline;
> +             curline->data = mallocstrcpy(NULL, buf);
> +             curline->lineno = ++lineno;
> +             curline->multidata = NULL;

Ow, what a waste...  You use an entire linestruct thing just to
store a single word from the dictionary?  :|

Anyway, okay for now.  But I don't think you need to set
multidata nor prev nor even lineno, as the completion code
doesn't use those fields.

Also, don't call this curline and tmpline -- they are not lines
but mere words.  Better use 'current_word' and 'new_word', or
something like that.

> +     if (pletion_line == openfile->fileage) {
> +             pletion_line = get_dict(shard);
> +             pletion_line = pletion_line == NULL ? openfile->fileage : 
> pletion_line;

This looks weird.  Better use a temporary variable, and only change
pletion_line when get_dictionary() doesn't return NULL.

By the way, why do you pass 'shard'?  It is not used anywhere in
that function.

But... this means that when a syntax specifies a dictionary, then nano
will only find possible completions in that dictionary and no longer
in the current file?

Oh, and please always send a complete patch, with commit message and
relevant explanations and signed-off line.

Benno

signature.asc
Description: OpenPGP digital signature