Message ID | 20200605124539.28749-1-tromey@adacore.com |
---|---|
State | New |
Headers | show |
Series | fortran/95509 - fix spellcheck-operator.f90 regression | expand |
Hi Tom, > My earlier patch to add case handling to the spell checker caused a > Fortran regression. I believe I must have misread the test results. > > This patch fixes the problem by changing the cutoff. I chose this > value because the previous patch effectively multiplied the result of > get_edit_distance by 2 (unless a case change is involved). OK. Thanks for the patch! Regards Thomas
On 5 June 2020 17:12:58 CEST, Thomas Koenig via Fortran <fortran@gcc.gnu.org> wrote: >Hi Tom, > >> My earlier patch to add case handling to the spell checker caused a >> Fortran regression. I believe I must have misread the test results. >> >> This patch fixes the problem by changing the cutoff. I chose this >> value because the previous patch effectively multiplied the result of >> get_edit_distance by 2 (unless a case change is involved). > >OK. Thanks for the patch! Yes, this is probably OK although Fortran is case- insensitive. I think we lowercase names coming in from the source (at least for the internal identifier nodes), so we should not be affected by any case change. thanks,
Bernhard> Yes, this is probably OK although Fortran is case- Bernhard> insensitive. I think we lowercase names coming in from the Bernhard> source (at least for the internal identifier nodes), so we Bernhard> should not be affected by any case change. I already checked it in, but FWIW it would be pretty easy to change get_edit_distance to have a case-insensitive mode. Tom
Hi Tom, > Bernhard> Yes, this is probably OK although Fortran is case- > Bernhard> insensitive. I think we lowercase names coming in from the > Bernhard> source (at least for the internal identifier nodes), so we > Bernhard> should not be affected by any case change. > > I already checked it in, but FWIW it would be pretty easy to change > get_edit_distance to have a case-insensitive mode. For Fortran identifiers, that would be really good. Would you do that? Regards Thomas
On Tue, 9 Jun 2020 08:14:55 +0200 Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Tom, > > > Bernhard> Yes, this is probably OK although Fortran is case- > > Bernhard> insensitive. I think we lowercase names coming in from the > > Bernhard> source (at least for the internal identifier nodes), so we > > Bernhard> should not be affected by any case change. > > > > I already checked it in, but FWIW it would be pretty easy to change > > get_edit_distance to have a case-insensitive mode. > > For Fortran identifiers, that would be really good. Would you do that? Is it really needed? We do not enter internal helper names (class names, anything artificial, anything leading underscore or leading uppercase) to the suggestions proposals so should never actually recommend any of those? And we lowercase identifiers from the user: $ printf "iIIi = 42\nend" | gfortran-10 -fimplicit-none -ffree-form -xf95 - <stdin>:1:4: Error: Symbol ‘iiii’ at (1) has no IMPLICIT type Why would we need a case-insensitive switch for the distance? TIA,
Hi Bernhard, >> For Fortran identifiers, that would be really good. Would you do that? > Is it really needed? > > We do not enter internal helper names (class names, anything > artificial, anything leading underscore or leading uppercase) to the > suggestions proposals so should never actually recommend any of those? You're quite right, this is not needed. Thanks for pointing this out :-) Regards Thomas
diff --git a/gcc/fortran/misc.c b/gcc/fortran/misc.c index 0fc25df8a22..46c6277c2b9 100644 --- a/gcc/fortran/misc.c +++ b/gcc/fortran/misc.c @@ -397,7 +397,7 @@ gfc_closest_fuzzy_match (const char *typo, char **candidates) likely to be meaningless. */ if (best) { - unsigned int cutoff = MAX (tl, strlen (best)) / 2; + unsigned int cutoff = MAX (tl, strlen (best)); if (best_distance > cutoff) {