diff mbox series

[2/2] keytable: Remove comments before processing keymaps

Message ID 20190701163813.25032-2-hadess@hadess.net (mailing list archive)
State New, archived
Headers show
Series [1/2] keytable: Add source information in generated keymaps | expand

Commit Message

Bastien Nocera July 1, 2019, 4:38 p.m. UTC
Do our best to remove comments from each line we process from the keymap
sources, so as to avoid commented duplicates and false positives
sneaking in to the keymap definitions.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 utils/keytable/gen_keytables.pl | 3 +++
 1 file changed, 3 insertions(+)

Comments

Sean Young July 2, 2019, 9:08 a.m. UTC | #1
On Mon, Jul 01, 2019 at 06:38:13PM +0200, Bastien Nocera wrote:
> Do our best to remove comments from each line we process from the keymap
> sources, so as to avoid commented duplicates and false positives
> sneaking in to the keymap definitions.
> 
> Signed-off-by: Bastien Nocera <hadess@hadess.net>
> ---
>  utils/keytable/gen_keytables.pl | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/utils/keytable/gen_keytables.pl b/utils/keytable/gen_keytables.pl
> index 3dc74ba6..d73daf58 100755
> --- a/utils/keytable/gen_keytables.pl
> +++ b/utils/keytable/gen_keytables.pl
> @@ -138,6 +138,9 @@ sub parse_file($$)
>  		}
>  
>  		if ($read) {
> +			# Remove comments
> +			~ s#/\*.*?\*/##sg;
> +			~ s#.*\*/##sg;

This doesn't solve the

	/* { 0x800ff40b, KEY_ENTER },
	   not used */

case. Or // comments.

>  			if (m/(0x[\dA-Fa-f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
>  				$out .= "$1 = \"$2$3\"\n";
>  				next;
> -- 
> 2.21.0
Bastien Nocera July 2, 2019, 9:29 a.m. UTC | #2
On Tue, 2019-07-02 at 10:08 +0100, Sean Young wrote:
> On Mon, Jul 01, 2019 at 06:38:13PM +0200, Bastien Nocera wrote:
> > Do our best to remove comments from each line we process from the
> > keymap
> > sources, so as to avoid commented duplicates and false positives
> > sneaking in to the keymap definitions.
> > 
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > ---
> >  utils/keytable/gen_keytables.pl | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/utils/keytable/gen_keytables.pl
> > b/utils/keytable/gen_keytables.pl
> > index 3dc74ba6..d73daf58 100755
> > --- a/utils/keytable/gen_keytables.pl
> > +++ b/utils/keytable/gen_keytables.pl
> > @@ -138,6 +138,9 @@ sub parse_file($$)
> >  		}
> >  
> >  		if ($read) {
> > +			# Remove comments
> > +			~ s#/\*.*?\*/##sg;
> > +			~ s#.*\*/##sg;
> 
> This doesn't solve the
> 
> 	/* { 0x800ff40b, KEY_ENTER },
> 	   not used */
> 
> case. Or // comments.

Those weren't problems I was aware of, or that I was trying to solve...

The presence of a "0x800ff40b = KEY_ENTER" won't stop the remote
definition from loading, or the remote to work correctly, and //
comments usually don't get merged into the kernel sources.

> >  			if (m/(0x[\dA-Fa-
> > f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
> >  				$out .= "$1 = \"$2$3\"\n";
> >  				next;
> > -- 
> > 2.21.0
Bastien Nocera July 2, 2019, 9:43 a.m. UTC | #3
On Tue, 2019-07-02 at 10:08 +0100, Sean Young wrote:
> On Mon, Jul 01, 2019 at 06:38:13PM +0200, Bastien Nocera wrote:
> > Do our best to remove comments from each line we process from the
> > keymap
> > sources, so as to avoid commented duplicates and false positives
> > sneaking in to the keymap definitions.
> > 
> > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > ---
> >  utils/keytable/gen_keytables.pl | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/utils/keytable/gen_keytables.pl
> > b/utils/keytable/gen_keytables.pl
> > index 3dc74ba6..d73daf58 100755
> > --- a/utils/keytable/gen_keytables.pl
> > +++ b/utils/keytable/gen_keytables.pl
> > @@ -138,6 +138,9 @@ sub parse_file($$)
> >  		}
> >  
> >  		if ($read) {
> > +			# Remove comments
> > +			~ s#/\*.*?\*/##sg;
> > +			~ s#.*\*/##sg;
> 
> This doesn't solve the
> 
> 	/* { 0x800ff40b, KEY_ENTER },
> 	   not used */

This isn't in the current Linus kernel.

> case. Or // comments.

And this isn't used anywhere in the kernel either.

Those are the 2 lines that would remove those comments but I can't test
it. Note that if this was done properly, instead of fixing the bugs we
encounter, the whole parsing would need to be redone.

+                       ~ s#/\*.*?##sg;
+                       ~ s#//.*##sg;

> >  			if (m/(0x[\dA-Fa-
> > f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
> >  				$out .= "$1 = \"$2$3\"\n";
> >  				next;
> > -- 
> > 2.21.0
Sean Young July 2, 2019, 11:44 a.m. UTC | #4
On Tue, Jul 02, 2019 at 11:43:39AM +0200, Bastien Nocera wrote:
> On Tue, 2019-07-02 at 10:08 +0100, Sean Young wrote:
> > On Mon, Jul 01, 2019 at 06:38:13PM +0200, Bastien Nocera wrote:
> > > Do our best to remove comments from each line we process from the
> > > keymap
> > > sources, so as to avoid commented duplicates and false positives
> > > sneaking in to the keymap definitions.
> > > 
> > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > ---
> > >  utils/keytable/gen_keytables.pl | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/utils/keytable/gen_keytables.pl
> > > b/utils/keytable/gen_keytables.pl
> > > index 3dc74ba6..d73daf58 100755
> > > --- a/utils/keytable/gen_keytables.pl
> > > +++ b/utils/keytable/gen_keytables.pl
> > > @@ -138,6 +138,9 @@ sub parse_file($$)
> > >  		}
> > >  
> > >  		if ($read) {
> > > +			# Remove comments
> > > +			~ s#/\*.*?\*/##sg;
> > > +			~ s#.*\*/##sg;
> > 
> > This doesn't solve the
> > 
> > 	/* { 0x800ff40b, KEY_ENTER },
> > 	   not used */
> 
> This isn't in the current Linus kernel.

We might as well solve this properly and handle all cases.

> > case. Or // comments.
> 
> And this isn't used anywhere in the kernel either.

That's wrong. They are used.

> Those are the 2 lines that would remove those comments but I can't test
> it. Note that if this was done properly, instead of fixing the bugs we
> encounter, the whole parsing would need to be redone.
> 
> +                       ~ s#/\*.*?##sg;
> +                       ~ s#//.*##sg;

If you are unable to test it, I will fix it.

> 
> > >  			if (m/(0x[\dA-Fa-
> > > f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
> > >  				$out .= "$1 = \"$2$3\"\n";
> > >  				next;
> > > -- 
> > > 2.21.0


Sean
Bastien Nocera July 2, 2019, 1:20 p.m. UTC | #5
On Tue, 2019-07-02 at 12:44 +0100, Sean Young wrote:
> On Tue, Jul 02, 2019 at 11:43:39AM +0200, Bastien Nocera wrote:
> > On Tue, 2019-07-02 at 10:08 +0100, Sean Young wrote:
> > > On Mon, Jul 01, 2019 at 06:38:13PM +0200, Bastien Nocera wrote:
> > > > Do our best to remove comments from each line we process from
> > > > the
> > > > keymap
> > > > sources, so as to avoid commented duplicates and false
> > > > positives
> > > > sneaking in to the keymap definitions.
> > > > 
> > > > Signed-off-by: Bastien Nocera <hadess@hadess.net>
> > > > ---
> > > >  utils/keytable/gen_keytables.pl | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/utils/keytable/gen_keytables.pl
> > > > b/utils/keytable/gen_keytables.pl
> > > > index 3dc74ba6..d73daf58 100755
> > > > --- a/utils/keytable/gen_keytables.pl
> > > > +++ b/utils/keytable/gen_keytables.pl
> > > > @@ -138,6 +138,9 @@ sub parse_file($$)
> > > >  		}
> > > >  
> > > >  		if ($read) {
> > > > +			# Remove comments
> > > > +			~ s#/\*.*?\*/##sg;
> > > > +			~ s#.*\*/##sg;
> > > 
> > > This doesn't solve the
> > > 
> > > 	/* { 0x800ff40b, KEY_ENTER },
> > > 	   not used */
> > 
> > This isn't in the current Linus kernel.
> 
> We might as well solve this properly and handle all cases.
> 
> > > case. Or // comments.
> > 
> > And this isn't used anywhere in the kernel either.
> 
> That's wrong. They are used.

Use in the keymap definitions? I don't think so. Either that or the 2
additional lines underneath here don't work. I think they do.

> > Those are the 2 lines that would remove those comments but I can't
> > test
> > it. Note that if this was done properly, instead of fixing the bugs
> > we
> > encounter, the whole parsing would need to be redone.
> > 
> > +                       ~ s#/\*.*?##sg;
> > +                       ~ s#//.*##sg;
> 
> If you are unable to test it, I will fix it.
diff mbox series

Patch

diff --git a/utils/keytable/gen_keytables.pl b/utils/keytable/gen_keytables.pl
index 3dc74ba6..d73daf58 100755
--- a/utils/keytable/gen_keytables.pl
+++ b/utils/keytable/gen_keytables.pl
@@ -138,6 +138,9 @@  sub parse_file($$)
 		}
 
 		if ($read) {
+			# Remove comments
+			~ s#/\*.*?\*/##sg;
+			~ s#.*\*/##sg;
 			if (m/(0x[\dA-Fa-f]+)[\s\,]+(KEY|BTN)(\_[^\s\,\}]+)/) {
 				$out .= "$1 = \"$2$3\"\n";
 				next;