Skip to content
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

Fix Reason detection #4473

Closed
wants to merge 1 commit into from
Closed

Conversation

dawee
Copy link

@dawee dawee commented Mar 28, 2019

  • I removed .re from C++ extensions.
  • I removed the group OCaml of Reason

Description

This change is linked to the comment I made here: #3949 (comment)

On the ungroup of OCaml: Reason might be an extension of OCaml, it has its own identity. I'm a Reason developer I'm not an OCaml developer at all. I don't even know how to read OCaml properly and I work with Reason everyday. I really want the ability to give a ReasonML its "Reason" identity so I can find them in github trendings.

On the removal of .re in C++: I've been C++ developer and I never saw this extension. I just checked on cppreference and I don't see it either. It's colliding with the official Reason extension.

The overall idea is to have our ReasonML projects having a "Reason" identity, so we can share as we do with all the other languages.

Checklist:

  • I am associating a language with a new file extension.
    • I have included a change to the heuristics to distinguish my language from others using the same extension.

@dawee dawee mentioned this pull request Mar 28, 2019
16 tasks
@pchaigno
Copy link
Contributor

On the ungroup of OCaml: Reason might be an extension of OCaml, it has its own identity. I'm a Reason developer I'm not an OCaml developer at all. I don't even know how to read OCaml properly and I work with Reason everyday. I really want the ability to give a ReasonML its "Reason" identity so I can find them in github trendings.

We might want to add this to #4291. @Alhadis What do you think?

On the removal of .re in C++: I've been C++ developer and I never saw this extension. I just checked on cppreference and I don't see it either. It's colliding with the official Reason extension.

.re was initially added as a C++ extension because of those files.

@Alhadis
Copy link
Collaborator

Alhadis commented Mar 29, 2019

@Alhadis What do you think?

I'm not familiar with either OCaml or Reason, but it certainly sounds like grouping them was a mistake:

I don't even know how to read OCaml properly and I work with Reason everyday.

@dawee
Copy link
Author

dawee commented Mar 29, 2019

We might want to add this to #4291. @Alhadis What do you think?

I think the issue is a very good initiative, maybe it would still be easier to do the job language by language?

.re was initially added as a C++ extension because of [those files]

The weirdest thing is that the extension has been added to the C++ ones in the same commit Reason was added in Linguist.
The examples you're referencing seem to be specific to this code generator: re2c. I don't know if it helps but at least removing it wouldn't mean removing an official C++ extension.

Is there a way through linguist to detect with both source content and extension?

@Alhadis
Copy link
Collaborator

Alhadis commented Mar 29, 2019

an official C++ extension.

Just an FYI: if a file extension has enough usage in-the-wild, it doesn't matter if it's "official" or not. If hundreds of repositories on GitHub associate an extension with a language, that's important enough for Linguist to consider, irrespective of whether a language deems it official.

@anmonteiro
Copy link

On the ungroup of OCaml: Reason might be an extension of OCaml, it has its own identity.

I don't agree with this. Reason is and has been OCaml from its inception. It's just a frontend to the OCaml compiler, insofar as after the first compiler pass you can't know whether your code was Reason or OCaml to begin with

Additionally, Reason works with OCaml dependencies, and I'll go as far as saying it needs OCaml dependencies, given how more widespread the OCaml ecosystem of libraries is in comparison to Reason.

I'm a Reason developer I'm not an OCaml developer at all.

You may see it that way, but that's not true. You're using the OCaml compiler therefore I consider you an OCaml developer :)

I don't even know how to read OCaml properly and I work with Reason everyday.

This is a personal shortcoming which IMO is orthogonal to this argument.

@dawee
Copy link
Author

dawee commented Mar 29, 2019

The community is disagreeing with this so I'm closing the pull request.

@dawee dawee closed this Mar 29, 2019
@dawee
Copy link
Author

dawee commented Mar 29, 2019

Sorry I closed it very fast because it created a big debate in the community, we may come back here but for now we need a consensus.

And @Alhadis for what I said about removing .re extension on C++, saying it's not official was pushy of me, I'm sorry. It make total sense that a lot of people using it is a good reason to keep it working.

@smorimoto
Copy link

Reason is definitely just a syntax extension of OCaml. but if your opinion is reasonable for this project, TypeScript should be recognized as JavaScript.

@alavkx
Copy link

alavkx commented Oct 23, 2019

Same deal for Elixir/Erlang—there's a precedent for this already. Not having detection for the language hurts the growth of the community. I can't see the downside.

@smorimoto
Copy link

Solved by #4713!

@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants