Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add encode and decode expression functions #2842

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

istnv
Copy link
Member

@istnv istnv commented Apr 21, 2024

Adds string encode/decode functions.

fromHex(val)
Decodes a Hex encoded string.
eg fromHex("436f6d70616e696f6e") gives "Companion"

toHex(val)
Hex encodes a string.
eg toHex("Companion") gives "436f6d70616e696f6e"

from64(val)
Decodes a Base64 encoded string.
eg from64("Q29tcGFuaW9uCg=="") gives "Companion"

to64(val)
Encodes a string to Base64.
eg to64("Companion") gives "Q29tcGFuaW9uCg=="

@istnv istnv marked this pull request as draft April 21, 2024 05:32
@istnv istnv marked this pull request as ready for review April 21, 2024 05:46
@dnmeid
Copy link
Member

dnmeid commented Apr 21, 2024

Any thoughts on the names of these functions? I think 'fromHex' and 'toHex' can easily be mixed up with a numerical conversion.
For base64 there are the standard functions 'atob' (ascii to base64) and 'btoa' (base64 to ascii).
Maybe we should use the same names for the same functionality and likewise 'htoa' and 'atoh', which are not part of ecma script but are used by several other libraries.

@Julusian
Copy link
Member

@dnmeid you make good points there.
I would prefer something more self descriptive than htoa and atoh. like hexToAscii, asciiToHex, base64Decode and base64Encode.

@istnv
Copy link
Member Author

istnv commented Apr 21, 2024

I think 'fromHex' and 'toHex' can easily be mixed up with a numerical conversion.

Number literals (0x10, 0b11) are already converted automatically.
The documentation explicitly states 'string'.

I was looking for descriptive, yet brief, function names.
The limited space in the GUI makes complex expressions extremely hard to work on.

This was a 'fun' to get working:

concat(`Soldering Iron`,$(internal:custom_si_off) < $(internal:time_unix)  ? '' :  concat(`\nAuto Off`,`
 @${secondsToTimestamp(max(0,$(internal:custom_si_off)-$(internal:time_unix)))}`) )

(A language I used in the '80s had built-in ATH and HTA functions for ascii-to-hex and hex-to-ascii.)

For base64 there are the standard functions 'atob' (ascii to base64) and 'btoa' (base64 to ascii).

These have been deprecated for a while.

@dnmeid
Copy link
Member

dnmeid commented Apr 21, 2024

@istnv don't get me wrong, I'm not talking about implementation, just about the names.
I'd be fine too with hexToAscii, asciiToHex, base64Decode and base64Encode.
I also would prefer if there was a good shorter variant, but a wise colleague of me once said: code is more often read than written, so readability is more important than writeability.
The size of the text input originates from a time when we had no expressions and some crazy guys maybe used one variable. If it is not convenient anymore, that's a different issue.

@istnv
Copy link
Member Author

istnv commented Apr 21, 2024

I am also trying to keep the names similar since they are nearly identical functions.
What about encodeHex, decodeHex, encodeBase64, decodeBase64? With appropriate documentation updates.
Or to be more precise 'encodeBase16`? (joke)
No one has asked for base64 (yet) but it was easy to add once I was working on the code.

There is definitely a need for a better input widget.

Since we now have a couple of reserved namespaces (internal, this) for variables, I'd like to see if $(internal:custom_var) could be reduced to $(custom:var)

@istnv
Copy link
Member Author

istnv commented Apr 22, 2024

Another idea. Basically expose Buffer.from(... and Buffer.to(...:
decode(str,enc) and encode(str,enc) where enc is one of hex, base64, binary, or any other node character encoding.

decode(str, enc)

Decodes an string from the passed encoding ('hex','base64')

eg decode("436f6d70616e696f6e","hex") gives "Companion"

encode(str, enc)

Encodes a string to the passed encoding ('hex','base64')

eg encode("Companion","hex") gives "436f6d70616e696f6e"

@istnv
Copy link
Member Author

istnv commented Apr 22, 2024

Since 'hex' is used to pass binary data, I've set decode to return a latin1 formatted string.
ascii strips any character above 0x7f and utf8 adds an escape sequence to them.

@Julusian
If the enc parameter is missing, which is preferred:

  • force latin1 (becomes a no-op for both encode/decode, might be confusing)
  • return string Missing encoding option

@dnmeid
Copy link
Member

dnmeid commented Apr 22, 2024

If enc is missing or invalid, I suggest to go with the same default as js does: utf8
Returning an error message in the return value can lead to unwanted behavior, e.g. if the original text to be decoded is identical with the error message.

@istnv
Copy link
Member Author

istnv commented Apr 23, 2024

... the same default as js does: utf8

For binary messages, I have encountered 'utf8' and 'ascii' breaking values above 0x7f.
This was problem for the generic-tcp-udp and generic-serial when node switched from a default of 'binary'/'latin1' to 'utf8'

A default of 'latin1' will cause less confusion since that will probably be the majority of uses.

If someone really wants 'utf8', they specify that as the encoding.

@istnv
Copy link
Member Author

istnv commented Apr 24, 2024

@Julusian
This should be ready to merge.

@istnv
Copy link
Member Author

istnv commented Apr 25, 2024

@Julusian I don't have write access. I'm OK with that, but cannot merge the PR myself.

@Julusian Julusian changed the title Add hex and base64 encode/decode feat: add encode and decode expression functions Apr 26, 2024
@Julusian Julusian merged commit c412561 into bitfocus:main Apr 26, 2024
3 checks passed
@Julusian Julusian modified the milestones: v3.develop, v3.3 Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants