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

Perl 5 code being mistaken for Perl 6 #3637

Closed
wchristian opened this Issue May 23, 2017 · 16 comments

Comments

Projects
None yet
9 participants
@wchristian

wchristian commented May 23, 2017

This is recognized as Perl 6, wrongly:


=head1 NAME

class

=cut

package PH;

use 5.010;

This is recognized as Perl 5:


=head1 NAME



=cut

package PH;

use 5.010;
@wchristian

This comment has been minimized.

Show comment
Hide comment
@wchristian

wchristian May 23, 2017

I don't get reliable results when i push this to github with changes, but this looks like it also is recognized as Perl 6, wrongly:

use 5.006;
use strict;
use warnings;

=head1

module

=cut

And removing module changes it to perl 5.

wchristian commented May 23, 2017

I don't get reliable results when i push this to github with changes, but this looks like it also is recognized as Perl 6, wrongly:

use 5.006;
use strict;
use warnings;

=head1

module

=cut

And removing module changes it to perl 5.

@wchristian

This comment has been minimized.

Show comment
Hide comment
@wchristian

wchristian May 23, 2017

Forgot to mention, in both examples the files were Word.pm.

wchristian commented May 23, 2017

Forgot to mention, in both examples the files were Word.pm.

@zoffixznet

This comment has been minimized.

Show comment
Hide comment
@zoffixznet

zoffixznet May 23, 2017

Looks like just swapping these conditionals to check for Perl 5 regex first will solve the issue.

And then the cases where the disambiguation gets it wrong, Perl 6 users always have the option to renaming the file to .pm6 extension.

zoffixznet commented May 23, 2017

Looks like just swapping these conditionals to check for Perl 5 regex first will solve the issue.

And then the cases where the disambiguation gets it wrong, Perl 6 users always have the option to renaming the file to .pm6 extension.

@kentfredric

This comment has been minimized.

Show comment
Hide comment
@kentfredric

kentfredric May 23, 2017

if /^\s*(?:use\s+v6\s*;|(?:\bmy\s+)?class|module)\b/.match(data)

^ That heuristic would false positive on anything using MooseX::Declare style perl-5 syntax anyway.

So yeah, +1 on inversion.

kentfredric commented May 23, 2017

if /^\s*(?:use\s+v6\s*;|(?:\bmy\s+)?class|module)\b/.match(data)

^ That heuristic would false positive on anything using MooseX::Declare style perl-5 syntax anyway.

So yeah, +1 on inversion.

@pchaigno

This comment has been minimized.

Show comment
Hide comment
@pchaigno

pchaigno May 27, 2017

Collaborator

We'd welcome a pull request with a test (test/test_heuristics.rb) to implement that change 😃

Collaborator

pchaigno commented May 27, 2017

We'd welcome a pull request with a test (test/test_heuristics.rb) to implement that change 😃

@smola

This comment has been minimized.

Show comment
Hide comment
@smola

smola Jul 31, 2017

Contributor

Is it really worth it to have Perl 6 separated from Perl? It seems the only case where linguist tries to determine language version, no Python 3, Java 6 or things like that, and the amount of misclassifications seems significant.

Contributor

smola commented Jul 31, 2017

Is it really worth it to have Perl 6 separated from Perl? It seems the only case where linguist tries to determine language version, no Python 3, Java 6 or things like that, and the amount of misclassifications seems significant.

@zoffixznet

This comment has been minimized.

Show comment
Hide comment
@zoffixznet

zoffixznet Jul 31, 2017

Is it really worth it to have Perl 6 separated from Perl

Yes, considering it's an entirely different language. It's like C vs C++, except with poorer name.

zoffixznet commented Jul 31, 2017

Is it really worth it to have Perl 6 separated from Perl

Yes, considering it's an entirely different language. It's like C vs C++, except with poorer name.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jul 31, 2017

Collaborator

@smola Perl 5 and Perl 6 are, as @zoffixznet put it, two entirely separate languages. To quote "Programming Perl":

That 6 is deceptive though; Perl 6 is really a “kid sister” language to Perl 5, and not just a major update to Perl 5 that version numbers have trained you to expect.

Collaborator

Alhadis commented Jul 31, 2017

@smola Perl 5 and Perl 6 are, as @zoffixznet put it, two entirely separate languages. To quote "Programming Perl":

That 6 is deceptive though; Perl 6 is really a “kid sister” language to Perl 5, and not just a major update to Perl 5 that version numbers have trained you to expect.

@tessarin

This comment has been minimized.

Show comment
Hide comment
@tessarin

tessarin Sep 30, 2017

Contributor

@pchaigno Do you know if this was in fact fixed? I've got a similar problem in a repository for a Perl 5 module which has its main file being incorrectly interpreted as Perl 6, making the repository also Perl 6 (funny enough, named p5-POD-Auto). Thanks!

Contributor

tessarin commented Sep 30, 2017

@pchaigno Do you know if this was in fact fixed? I've got a similar problem in a repository for a Perl 5 module which has its main file being incorrectly interpreted as Perl 6, making the repository also Perl 6 (funny enough, named p5-POD-Auto). Thanks!

@pchaigno

This comment has been minimized.

Show comment
Hide comment
@pchaigno

pchaigno Oct 13, 2017

Collaborator

@tessarin No, I don't think a pull request for the above proposed change was submitted.

Collaborator

pchaigno commented Oct 13, 2017

@tessarin No, I don't think a pull request for the above proposed change was submitted.

@tessarin

This comment has been minimized.

Show comment
Hide comment
@tessarin

tessarin Oct 16, 2017

Contributor

@pchaigno May I submit one then? You mentioned adding a test to test/test_heuristics.rb, however there is already a test for loading samples and disambiguate Perl | Perl 6 (lines 224-231). In this case, adding a minimal example to samples/Perl (like the ones posted here) causes tests to fail. With the change in heuristics.rb they pass again. So, is adding the sample enough?

Contributor

tessarin commented Oct 16, 2017

@pchaigno May I submit one then? You mentioned adding a test to test/test_heuristics.rb, however there is already a test for loading samples and disambiguate Perl | Perl 6 (lines 224-231). In this case, adding a minimal example to samples/Perl (like the ones posted here) causes tests to fail. With the change in heuristics.rb they pass again. So, is adding the sample enough?

@pchaigno

This comment has been minimized.

Show comment
Hide comment
@pchaigno

pchaigno Oct 16, 2017

Collaborator

May I submit one then?

@tessarin Sure, go ahead 😄

You mentioned adding a test to test/test_heuristics.rb, however there is already a test for loading samples and disambiguate Perl | Perl 6 (lines 224-231).

Oh, right. In that case, yes, a sample is enough. Please add it in test/fixtures/Perl/ rather than in samples/ though. samples/ files should originate from "real" application. test/fixtures/ is for this kind of test-only example files.

Collaborator

pchaigno commented Oct 16, 2017

May I submit one then?

@tessarin Sure, go ahead 😄

You mentioned adding a test to test/test_heuristics.rb, however there is already a test for loading samples and disambiguate Perl | Perl 6 (lines 224-231).

Oh, right. In that case, yes, a sample is enough. Please add it in test/fixtures/Perl/ rather than in samples/ though. samples/ files should originate from "real" application. test/fixtures/ is for this kind of test-only example files.

@sidyll

This comment has been minimized.

Show comment
Hide comment
@sidyll

sidyll Oct 22, 2017

Any chance that this will get fixed soon?

sidyll commented Oct 22, 2017

Any chance that this will get fixed soon?

@tessarin

This comment has been minimized.

Show comment
Hide comment
@tessarin

tessarin Oct 22, 2017

Contributor

@sidyll well the pull request was already made and it was approved, but I'm not sure about how the review process works or the release frequency.

Contributor

tessarin commented Oct 22, 2017

@sidyll well the pull request was already made and it was approved, but I'm not sure about how the review process works or the release frequency.

@lildude lildude closed this in 21babbc Oct 23, 2017

@lildude

This comment has been minimized.

Show comment
Hide comment
@lildude

lildude Oct 23, 2017

Member

PR has been merged. I've been AFK for a week so need to do some catching up before I can make a new release. I can't say when this will be right now though.

Member

lildude commented Oct 23, 2017

PR has been merged. I've been AFK for a week so need to do some catching up before I can make a new release. I can't say when this will be right now though.

@tessarin

This comment has been minimized.

Show comment
Hide comment
@tessarin

tessarin Oct 24, 2017

Contributor

Thank you @lildude!

Contributor

tessarin commented Oct 24, 2017

Thank you @lildude!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment