racodond / sonar-gherkin-plugin

SonarQube Cucumber Gherkin Analyzer
GNU Lesser General Public License v3.0
36 stars 21 forks source link

Implements i18n #41

Closed ghost closed 7 years ago

ghost commented 7 years ago

Gherkin contains many lang ;-)

in this file

https://github.com/racodond/sonar-gherkin-plugin/blob/master/gherkin-frontend/src/main/java/org/sonar/gherkin/parser/GherkinLexicalGrammar.java

it will be useful to use this JSON as a keywords definition from this

https://github.com/cucumber/gherkin-java/blob/master/src/main/resources/gherkin/gherkin-languages.json

racodond commented 7 years ago

Gherkin contains many lang ;-)

I know :-)

in this file

https://github.com/racodond/sonar-gherkin-plugin/blob/master/gherkin-frontend/src/main/java/org/sonar/gherkin/parser/GherkinLexicalGrammar.java

it will be useful to use this JSON as a keywords definition from this

https://github.com/cucumber/gherkin-java/blob/master/src/main/resources/gherkin/gherkin-languages.json

Thanks for the pointer!

I first wanted to craft a version that supports well my needs. That's why it is limited to English for now. I'm almost done with it (will be done when version 1.2 is released, likely next week). Then, I'll tackle internationalization:

ghost commented 7 years ago

@racodond if you write a simple COTRIBIUTORS.md - iam and @nixel2007 can create pull request for you with our version of implementation for this ticket.

nixel2007 commented 7 years ago

@allustin please note, that we can't use native Gherkin java library, as we did it in our version of plugin. This one needs to improve lexer itself.

racodond commented 7 years ago

Great news if you feel like contributing! Before I find some time to create a CONTRIBUTORS.md file, just create a pull request and I'll review it.

racodond commented 7 years ago

@allustin @nixel2007 Here's a snapshot supporting i18n: https://github.com/racodond/sonar-gherkin-plugin/releases/tag/i18n It would be great if you could give it a try and provide some feedback. Thanks!

nixel2007 commented 7 years ago

@racodond good to hear you've done it fast! I think, we can try new version tomorrow (we are UTC+3), we have plenty of ru-features

racodond commented 7 years ago

Cool! Looking forward to getting your feedback!

nixel2007 commented 7 years ago

@racodond it works like a charm! image A few comments - it's common way to name feature files in native language. I think the gherkin:S1578 rule should be feeded with more permissable chars.

Also looks like your highlighter doesn't highlight comments properly. Is this Tree.kind correct https://github.com/racodond/sonar-gherkin-plugin/blob/master/gherkin-frontend/src/main/java/org/sonar/gherkin/visitors/SyntaxHighlighterVisitor.java#L94 ?

racodond commented 7 years ago

@nixel2007

it works like a charm!

Cool! Thanks for your feedback!

Also looks like your highlighter doesn't highlight comments properly

The highlighter is working fine on comments. Actually, I don't consider language declarations as comments. That's why it is not highlighted as a comment in your screenshot. Anyway, I just updated the plugin to add a specific highlighting for language declarations (green). See https://github.com/racodond/sonar-gherkin-plugin/commit/ed6f736d14c699bd75044147c5476204fa209848

image

A new snapshot is available with this fix at https://github.com/racodond/sonar-gherkin-plugin/releases/tag/1.3-RC1

it's common way to name feature files in native language. I think the gherkin:S1578 rule should be feeded with more permissable chars

I prefer to leave the rule as it is. It should be the user's responsibility to set the right regular expression parameter depending on their needs, language, etc.

nixel2007 commented 7 years ago

@racodond i have a parser error in features with tags and "Функционал" keyword (feature-keyword)


# encoding: utf-8
# language: ru

#https://github.com/silverbulleters/vanessa-behavior/issues/237

@IgnoreOnCIMainBuild

Функционал: Тестовая фича, проверяющая генерацию снипета по структуре сценария

Структура сценария: Привязка файлов сканов актов к документам ЭСТИ
    Допустим к заказу связанному с реализацией товаров и услуг <Номер> от <ДатаДокумента>  присоединен файл <ИмяФайла> только один

    Примеры:
        | ТомХраненияПервички   | МесяцГода   | Реализации | АктИсх  | ИмяФайла     | Номер | ДатаДокумента | 
        | ПутьКТому             | 2015.06     | Реализации | Акт исх | ИмяФайла.pdf | 524   | 2015.06.30    |       

can you check this?

Error message

Parse error at line 10 column 1: 1: 2: # encoding: utf-8 3: # language: ru 4: 5: #https://github.com/silverbulleters/vanessa-behavior/issues/237 6: 7: @IgnoreOnCIMainBuild 8: 9: 10: Функционал: Тестовая фича, проверяющая генерацию снипета по структуре сценария ^ 11: 12: 13: 14: Структура сценария: Привязка файлов сканов актов к документам ЭСТИ 15: Допустим к заказу связанному с реализацией товаров и услуг <Номер> от <ДатаДокумента> присоединен файл <ИмяФайла> только один 16: 17: 18: Примеры: 19: | ТомХраненияПервички | МесяцГода | Реализации | АктИсх | ИмяФайла | Номер | ДатаДокумента | 20: | ПутьКТому | 2015.06 | Реализации | Акт исх | ИмяФайла.pdf | 524 | 2015.06.30 |

Not yet updated to the 1.3-RC1, sorry. Checket at i18n git tag

racodond commented 7 years ago

@nixel2007

I check that the language declaration as the first line of a file. It is on the second line in your example. Thus, the file is considered as being written in the default language (en). Hence the parse error.

Can language declaration be on the second line? How about the encoding declaration? Could you please point to some documentation defining where language, encoding, etc. declarations are supposed to be?

Thanks

David

nixel2007 commented 7 years ago

@racodond the official spec says, that language declaration must be at first line of the file https://github.com/cucumber/cucumber/wiki/Spoken-languages

however the Feature Introduction says that there can be encoding comment also - https://github.com/cucumber/cucumber/wiki/Feature-Introduction

I think you could read all comments until any non-comment with text, check the language in this comments and if you found it - feed the parser with given lang.

racodond commented 7 years ago

@nixel2007 I'll find out a way to support language and encoding whatever the order... after Christmas.

nixel2007 commented 7 years ago

@racodond sure, thanks a lot! Merry Christmas :)

racodond commented 7 years ago

@nixel2007 Thanks! I just had a look at https://github.com/cucumber/gherkin and I can't find any piece of code dealing with encoding. It looks like this is not supported even if it is written in one single (of many) documentation. Are you sure that it is really taken into account in your feature file? What if you set it to something else like "blabla"? Does it really change the behavior? Or is it just ignored and interpreted as a simple comment?

nixel2007 commented 7 years ago

@racodond in most cases we use our own implemantation of Gherkin parser and feature exec (for 1C:Enterprise 8), and no, we just interpretate it as a comment. originaly parser was based on Gherkin v3, maybe cucumber team just removed it in v4. We can change our instrument to not generate this #encoding line on new features, but we already have a lot (thousands on hundreds of clients) feature-file with this line, so i'll be much obliged if gherkin-plugin will also interprete it as comment. Maybe we can add some new rule, lets say language must be defined on only at first line in case parser get it at any other line than first.

racodond commented 7 years ago

@nixel2007 I'm not really keen on adding support of encoding. I'd prefer that you change your "instrument" to no longer generate this encoding line. And talk with your clients to proceed with a search and replace to remove those lines.

nixel2007 commented 7 years ago

@racodond Ok! Thank you anyway, it works and works good!

racodond commented 7 years ago

@nixel2007 Thanks for your understanding and the kind words!