Implements i18n #41

Closed
allustin opened this Issue Dec 14, 2016 · 19 comments

Projects

None yet

3 participants

@racodond
Owner
racodond commented Dec 14, 2016 edited

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:

  • Grammar (this ticket)
  • Spell checker (#42)
  • Update of rule "Source code should be properly indented" that is tight to English keywords for now. See #43
@racodond racodond added this to the 1.3 milestone Dec 14, 2016
@racodond racodond changed the title from implements i18n to Implements i18n Dec 14, 2016
@allustin
allustin commented Dec 15, 2016 edited

@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

@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
Owner

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 racodond added a commit that closed this issue Dec 19, 2016
@racodond Fix #41 Implements i18n 7375313
@racodond racodond closed this in 7375313 Dec 19, 2016
@racodond racodond added a commit that referenced this issue Dec 19, 2016
@racodond Fix #41, Fix #42, Fix #43 Support all Gherkin dialects (i18n)
Fix #41, Fix #42, Fix #43 Support all Gherkin dialects (i18n)
7f2bd93
@racodond
Owner

@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
nixel2007 commented Dec 19, 2016 edited

@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
Owner

Cool! Looking forward to getting your feedback!

@nixel2007
nixel2007 commented Dec 22, 2016 edited

@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
Owner

@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 ed6f736

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
nixel2007 commented Dec 22, 2016 edited

@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
Owner

@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

@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
Owner

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

@nixel2007

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

@racodond
Owner

@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
nixel2007 commented Dec 24, 2016 edited

@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
Owner
racodond commented Dec 25, 2016 edited

@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

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

@racodond
Owner

@nixel2007 Thanks for your understanding and the kind words!

@allustin allustin added a commit to silverbulleters/sonar-1c-bsl-public that referenced this issue Dec 25, 2016
@allustin allustin заменена ссылка на плагин Gherkin
теперь можно и коллективно по развивать racodond/sonar-gherkin-plugin#41 (comment)
e2b3c47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment