code blocks appearing as normal text

[brunakov]

ETH Ticker for example is configured as:

  eth:
    regex:             "^eth$|^eth .*$|^ethereum$|^eth.sh$"
    cmd:               eth.sh
    help:              gives Ethereum price info
    markdown_convert:  false
    formatted:         true
    code:              true

however the output is :

I would expect this to appear as a code block since code is set to true i.e. :

[troglodyne]

Seeing this myself on my own home server. Most importantly this makes the output of the weather builtin script look quite bad. My current speculation is that this could be a bug from the client library in use (matrix-nio) or simply changes in the element client layer. Unfortunately, it appears that simply modifying one's commands.yaml to set code=false then send it as markdown along with the appropriate markdown modifiers (code fence) also fails to produce the desired outcome (instead producing the output verbatim, but not in a monospaced font). Attempts to pass in raw HTML to produce a code block after setting formatted=false surrounding this also fails to produce the desired outcome. I'll see about debugging it tonight since I have some free time.

Some details about my setup follow.

Client:

• Element version: 1.7.24
• olm version: 3.2.1
• matrix-nio: 0.17.0

Server:

• matrix-synapse 1.11.0-1
• Ubuntu 20.04.1

matrix-eno-bot version sha: c67220057728988e035557709f333fb1f92b9cfc

[troglodyne]

When viewing the source of the messages, it became immediately clear that the format property of the message blob is not set for the affected messages (as well as the formatted_body). The code looks like it should be setting this given the configuration, but something must be going haywire for it not to be so.

Will grab a dump next from chat_commands.py to see whether it is a code issue with this project or whether the server is simply discarding the extra properties for some reason here.

[troglodyne]

Dang, looks like it's the code (added info print to get what content looked like). Guessing the values in YAML aren't translating to what I would think they are

2021-04-08 01:33:03,568 | chat_functions [INFO] {'body': 'houston: Partly cloudy +25°C (+26°C)  0.1mm', 'msgtype': 'm.notice'}

YAML looking like I'd expect given the example file, could be I need True and False instead literally?

  weather:
    regex: "^weather$|^tiempo$|^wetter$|^temps$|^weather .*$|^tiempo .*$|^eltiempo .*$|^wetter .*$|^temps .*$|^weather.sh$|^weather.sh .*"
    cmd: weather.sh
    help: give weather forecast
    markdown_convert: false
    formatted: true
    code: true

[troglodyne]

Looks like if the values are set in config, the result is that property is always False in send_text_to_room. This certainly explains why help is formatted correctly whereas the custom commands appear not to be.

[troglodyne]

Added debug print to command_dict.py to get what self.command_dict["commands"][command]["formatted"] actually is

Turns out == on the property is probably not the best idea, as the YAML parser reads true and false properly, negating the need for a "true" string literal check.

Fix is to use not not if the goal is boolean coercion to prevent user footgunning. PR incoming soon

[troglodyne]

PR submitted, let me know if y'all need me to follow up further :)

@atomtm Does the code change in 56deb2caf6c1505d4441e4bae06ebd73a6e17a8e fix it for you? Worked like a charm for me.

[brunakov]

PR submitted, let me know if y'all need me to follow up further :)

@atomtm Does the code change in 56deb2c fix it for you? Worked like a charm for me.

this worked with :

"""
        if "markdown_convert" in self.command_dict["commands"][command].keys():
            return not self.command_dict["commands"][command]["markdown_convert"]
        else:
            return CommandDict.DEFAULT_OPT_MARKDOWN_CONVERT

@@ -232,7 +232,7 @@ def get_opt_formatted(self, command):
        """
        if "formatted" in self.command_dict["commands"][command].keys():
            return not self.command_dict["commands"][command]["formatted"]
        else:
            return CommandDict.DEFAULT_OPT_FORMATTED

@@ -245,7 +245,7 @@ def get_opt_code(self, command):
        """
        if "code" in self.command_dict["commands"][command].keys():
            return not self.command_dict["commands"][command]["code"]
        else:
            return CommandDict.DEFAULT_OPT_CODE

changed not not to not

[troglodyne]

While that's somewhat... interesting logically, I'm glad that at least fixed it for you.

Only thing this makes me think is that I may want to look at adding test cover for the impacted code due to the inconsistency in user experience just to make sure my double negation approach to bool coercion works properly

[teodesian]

if the existing value in config is "false" rather than an actual false value coercing with 'not not' would do the wrong thing.

The right thing to do to account for existing config data would be to ternary around 'true' and 'false', call bool() otherwise (python has 1 edge case where !! is the wrong coercion iirc)

[troglodyne]

I mean, yes, any string will be converted to True in the case of not not. That said, the commands.yaml.example does not contain this, it contains true or false (not "true" or "false"), so I would hope the user would at least have to go out of their way to get things like that.

[8go]

Thank you all 3 @atomtm @troglodyne and @teodesian for contributing and bringing up this issue and finding solutions.

OK, yes, I confirm, there is a bug. The bug is because I thought Yaml would return a string type when it finds a value "true" (or "false") in the Yaml config file. Yaml is smart, and it actually returns a boolean. So the actual code line return self.command_dict["commands"][command]["markdown_convert"] == "true" is definitely a bug as it compares a boolean in the dict to a string. @troglodyne fixed the bug by changing it to return not not self.command_dict["commands"][command]["markdown_convert"]. He did the not not to force boolean, but that is not needed as Yaml retruns boolean out of the box. So, return self.command_dict["commands"][command]["markdown_convert"] is actually correct, in my humble opinion.

But, @atomtm put just a single not. That makes no sense to me. I cannot see how that works.

Also, very important to note is that there is a bug in Element (I reported it a LONG time ago, maybe 1 year ago) where on Element Android, the "code" instruction is NOT working. Android will never show anything as a code block! As a result 2 recommendations:

• test "code" block related stuff on Firefox (and NOT on Android)
• kindly and nicely "complain" to Element so they eventually fix this Element bug

If there are no new input/feedback, I will await for @troglodyne to change return not not self.command_dict["commands"][command]["markdown_convert"] to return self.command_dict["commands"][command]["markdown_convert"] and then merge his PR.

Sounds good to everybody?

[8go]

see also https://github.com/vector-im/element-android/issues/2328

[brunakov]

Related to Element Android, if you send a code block from Element Desktop this is rendered correctly on Element Android. But not if the code block is sent by the bot.

[8go]

Related to Element Android, if you send a code block from Element Desktop this is rendered correctly on Element Android. But not if the code block is sent by the bot.

That info is useful. I will investigate that. Thank you @atomtm I will try to fix this next week.

[8go]

Related to Element Android, if you send a code block from Element Desktop this is rendered correctly on Element Android. But not if the code block is sent by the bot.

That info is useful. I will investigate that. Thank you @atomtm I will try to fix this next week.

I found a work-around that makes it work also on Android! See bug fix commit in https://github.com/8go/matrix-eno-bot/commit/8cc46277e3bc688a3b17c233fc4635ee4ac0b3ad This is specific to Android.

I also updated https://github.com/vector-im/element-android/issues/2328

[troglodyne]

Sounds good, will update PR shortly to remove the unnecessary type coercion

[8go]

Thanks and :clap: everybody, @atomtm @teodesian @troglodyne Should be resolved now, closing issue.