Skip to content

Test na null a potenciální fix UTF-8 filtrů #2

Closed
wants to merge 2 commits into from

3 participants

@borekb
borekb commented Mar 10, 2012

Nejsem si úplně jistý, jakou roli plní UTF-8 kódování a dekódování, ale i po odstranění těchto filtrů testy fungují, takže jsem vše hodil do commitu.

borekb added some commits Mar 10, 2012
@borekb borekb Removed UTF-8 encoding/decoding and the letter "Â" from a pattern ins…
…ide removeSoftHyphens(). Not sure why it was there but the tests still pass with this simpler implementation.
c4bccdb
@borekb borekb Can process null input 9579637
@rarous
Owner
rarous commented on 9579637 Mar 11, 2012

Proč by texy měla zpracovávat null? A co undefined? BTW v TDD se řeší hlavně happy day scénáře. Takže řešit null na vstupu v této fázi je premature.

Komentář OK, jen se zeptám: zamítáš tenhle commit, protože je součástí pull requestu, který obsahuje změnu na UTF-16 (kterou chápu, že je potřeba odmítnout), nebo odmítáš přijmout kontrolu na null, i kdyby byla v pull requestu sama?

Owner

Ano, odmítám i tento commit. :) Nepovažuju vracení prázdného stringu na null jako správné chování. Null má svoji semantiku a ta by měla být zachována. Texy.js v tuto chvíli null ani undefined řešit neumí a padá na nich, což je IMO korektní chování.

Pokud by sis rozmyslel, že tento commit odmítáš z principu (Texy.js má spadnout na null), dej vědět, přeimplementoval bych tento commit, aby byl technicky v pořádku.

Owner

Borku, null je něco, co se v JS skoro nevyskytuje. Null musí někdo vytvořit explicitně. Fakt je to case, kterým se nemá cenu zabývat.

Nejde jen o null. Jak jsi třeba psal, je třeba otázka, co má Texy.js udělat s process(0). Právě proto, že se mezními případy nechceš zabývat, ti nabízím, že tu práci udělám. Ale nebudu ji dělat, pokud ji odmítneš, tak proto se ptám. Je to tvoje repo.

Owner

Zatím v tom nevidím žádnou value, až to bude problém při užívání, rád takovou změnu přijmu, do té doby je to zabitej čas, kterej se může věnovat přínosnějším věcem.

@rarous
Owner
rarous commented on c4bccdb Mar 11, 2012

Originální implementace Texy! pracuje v UTF8, ECMAScript pracuje v UTF16. Kvůli kompatibilitě a přenositelnosti jsem se rozhodl, že Texy.js bude taky pracovt v UTF8.

@rarous rarous closed this Mar 11, 2012
@stej
stej commented Mar 12, 2012

Nějako nerozumím, proč je potřeba udržovat tuto kompatibilitu s Texy! Pokud to dobře chápu, je to implementace v PHP. Pokud by někdo použil TexyJs na klientovi a Texy! na serveru, stejně to půjde přes web. Na serveru zřejmě TexyJs a Texy! spolu spouštěny nebudou. Můžeš to nějak vyjasnit?

@rarous
Owner
rarous commented Mar 12, 2012

Jednoduše je to lenost. Chci mít možnost jednoduše překlápět existující patterny. Nevylučuju, že pak konverzi na UTF-8 zrušim a patterny přepíšu, ale to až budu mít pořádnou baterii testů. Dřív ne.

@borekb
borekb commented Mar 12, 2012

Pořádnou baterii testů máš neustále. Ten druhý důvod beru.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.