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

Better language detection #68

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gabegorelick
Copy link
Collaborator

Closes #22, #40, and probably others

Closes rubenv#22, rubenv#40, and probably others
@gabegorelick
Copy link
Collaborator Author

Is there a reason we don't just parse everything that's not JS as HTML? Currently, some tests expect files to be skipped if they have an unknown extension, but treating everything as HTML would simplify lots of things, and is probably just a NOOP if it doesn't have any SGML in it. Or am I missing something?

@rubenv
Copy link
Owner

rubenv commented Dec 9, 2014

I don't mind the current approach of manually adding test cases: it ensures that what we promise to support will stay supported.

@gabegorelick
Copy link
Collaborator Author

I don't mind the current approach of manually adding test cases: it ensures that what we promise to support will stay supported.

That's fine, but right now we have to manually add support for every arbitrary HTML templating language that pops up. What's wrong with treating everything as HTML by default, and then adding test cases as we go?

@rubenv
Copy link
Owner

rubenv commented Dec 3, 2015

Oh crap, it's been a year this has been gathering dust, doh!

If I understand it correctly, we try to detect the language correctly, with extensions being an override map?

That actually makes sense.

@rubenv
Copy link
Owner

rubenv commented Dec 3, 2015

What's wrong with treating everything as HTML by default, and then adding test cases as we go?

Sounds reasonable!

@gabegorelick
Copy link
Collaborator Author

If I understand it correctly, we try to detect the language correctly, with extensions being an override map?

That's the idea. We could maybe use a test to make sure overriding extensions still works if there isn't one already in the repo.

The detection discerns whether the source is a "markup" language, which covers most HTML-like languages. We also manually check the extension for "html", which catches a few other cases (hopefully that shouldn't cause any false positives.

@rubenv
Copy link
Owner

rubenv commented Dec 21, 2015

I tried merging this, but I can't seem to get it working again after resolving the conflicts.

First (solveable) problem is that it doesn't recognize JSP / Tapestry files. Easily fixed by adding an override in extensions (though there's probably a better way to fix that).

More bizarre is that I get these failures:

  1) Extract Merges multiple views into one .pot:

      AssertionError: 4 == 2
      + expected - actual

      -4
      +2

      at Context.<anonymous> (test/extract.js:30:16)

  2) Extract Merges duplicate strings with references:

      AssertionError: 4 == 2
      + expected - actual

      -4
      +2

      at Context.<anonymous> (test/extract.js:45:16)

Any idea why that might suddenly happen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants