Re: Use a hash table for the lexer keywords (issue 549920043 by address@hidden)

[dak]

On 2020/05/01 11:15:29, hanwenn wrote:
> On 2020/04/30 07:58:09, dak wrote:
> > On 2020/04/30 07:42:16, hahnjo wrote:
> > > On 2020/04/27 11:58:11, dak wrote:
> > > > Tracker issue: 5946
> (https://sourceforge.net/p/testlilyissues/issues/5946/)
> > > > Rietveld issue: 577840053
(https://codereview.appspot.com/577840053)
> > > > Issue description:
> > > >   Use Scheme_hash_table for keyword handling
> > > 
> > > We should probably decide between these two approaches, we likely
can't do
> > both.
> > > I see the advantage of using SCM values for everything, but I'm
not familiar
> > > with the code.
> > 
> > I think it makes more sense here not to introduce new data types for
a job
> that
> > is inherent to Scheme's operation.  Having both patches on countdown
at the
> same
> > time obviously does not make sense: if discussion is required
(Han-Wen?), it
> > would make sense to stop both until this is resolved.
> > 
> > An independent component is the removal of ly:lexer-keywords . 
There is no
> > indication that it ever has been used; it is cheap to provide with
my version.
> 
> > However, revisiting its code I also see that it takes a lexer as an
argument. 
> > My patch, like Han-Wen's, stops lexers from having their individual
keytable
> (an
> > implementation detail that was never used for any purpose).  So even
if the
> > function were retained, letting it take an argument, while making
for
> backwards
> > compatibility, does not appear to make sense.  This could be
addressed in a
> > separate patch/issue.  Or it could be removed in a separate
patch/issue.
> > 
> > One possible use for it would be using LilyPond itself for
generating syntax
> > highlighting for editors.  The current solutions rather extract
stuff from the
> > source I seem to remember.
> 
> 1) we can take dak's patch if you prefer. They're roughly equivalent
anyway.
> 
> 2) I suppose ly:lexer-keywords was for editor modes, but it hasn't
been used for
> that.
> Updating the scripts to generate the modes is extra work, so I vote
for removing
> it. If 
> nobody bothered in the last 13 years, it can't be that important.

I'll make the removal a separate issue (and obviously commit) once the
"base patch" is in.  That makes it possible to find, reference, and
possibly revert the removal in case that is ever wanted.

https://codereview.appspot.com/549920043/