Karuha icon indicating copy to clipboard operation
Karuha copied to clipboard

[Drafty] Inconsistency with other implementations

Open tututumuru opened this issue 1 year ago • 6 comments

  • https://github.com/Visecy/Karuha/blob/master/karuha/text/drafty.py#L38

I understand that the Drafty format is written very poorly. But all implementations allow to pass such format:

{"ent":null,"fmt":null,"txt":"Some text"}

Your implementation has an error:

Input should be a valid list [type=list_type, input_value=None, input_type=NoneType]

👀

tututumuru avatar Feb 24 '25 14:02 tututumuru

fmt + ent re nullable:

  • https://github.com/tinode/chat/blob/master/server/drafty/drafty.go#L37
  • https://github.com/tinode/tindroid/blob/master/tinodesdk/src/main/java/co/tinode/tinodesdk/model/Drafty.java#L159
  • https://github.com/tinode/ios/blob/master/TinodeSDK/model/Drafty.swift#L90
  • https://github.com/tinode/tinode-js/blob/master/src/drafty.js#L758

tututumuru avatar Feb 24 '25 14:02 tututumuru

I'll consider changing that. However, is this possible in real occasions? Would such null values ​​actually be included in the drafty that is actually sent?

Ovizro avatar Feb 24 '25 15:02 Ovizro

It seems like the easiest way to fix it:

class Drafty(BaseModel):
    txt: str = ''
    fmt: List[DraftyFormat] = Field(default_factory=list)
    ent: List[DraftyExtend] = Field(default_factory=list)

    @field_validator('fmt', 'ent', mode='before')
    @classmethod
    def replace_none_fmt_ent(cls, value):
        if value is None:
            return []

        return value

This is just a guess. It works on basic tests only.

tututumuru avatar Feb 24 '25 15:02 tututumuru

I'll consider changing that. However, is this possible in real occasions? Would such null values ​​actually be included in the drafty that is actually sent?

Good question, if your client side does not have such behavior, then it is not critical. In my case it is not so (too lazy to ask to fix it) 🙂

tututumuru avatar Feb 24 '25 15:02 tututumuru

Although I'm sure there is a client that can send something like this:

Drafty.model_validate({'ent': [{'tp': "LN", "data": {"some": 123}}], 'fmt': None, 'txt': 'Some text'})
# or ent is null

And will be error in the current code.

tututumuru avatar Feb 24 '25 15:02 tututumuru

Well, from all perspectives, fixing it is the best option. If you have the time and willingness to submit a PR to fix this, I'd be very happy.

Ovizro avatar Feb 24 '25 16:02 Ovizro