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

expose autocannonize func for keyexpr wrapper #101

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Charles-Schleich
Copy link
Member

Add Key Expr Wasm functionality.

  • Expose autocanonize function in in Key Expr Class

Copy link

PR missing one of the required labels: {'breaking-change', 'bug', 'dependencies', 'documentation', 'enhancement', 'internal', 'new feature'}

@Charles-Schleich Charles-Schleich added the enhancement New feature or request label Jan 20, 2025
/*
* Returns true if the keyexprs intersect, i.e. there exists at least one key which is contained in both of the sets defined by self and other.
*/
autocanonize(other: IntoKeyExpr): KeyExpr {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is incorrect. Also this should be free function, it doesn't use "this".
Probably this require to make "call_wasm" free function too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or make this method static, ts allows this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call_wasm is just a function that is part of the keyexpr class, to abstract away return types and function calls.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just wondering why autocanonize is a method of already created KeyExpr? This function should accept string and create key new expression. I.e. this is a constructor. It can be implemented like this:

class KeyExpr {
...
	public static autocanonize(s: string): KeyExpr | null {
		const key_expr = ??? call_wasm<String>(s, autocanonize);
                // we should be able to call wasm without `this`
		if key_expr.is_ok() { // not sure how to really check it to ok
                   return new KeyExpr(key_expr)
                } else {
                   return null; // TODO: it is the way how we fail in zenoh_ts? Need to check
                }
	}
}

@@ -6,6 +6,10 @@ export function join(a: number, b: number, c: number, d: number, e: number): voi
export function concat(a: number, b: number, c: number, d: number, e: number): void;
export function includes(a: number, b: number, c: number, d: number, e: number): void;
export function intersects(a: number, b: number, c: number, d: number, e: number): void;
<<<<<<< HEAD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

garbage in file, build is passed only because this file is regenerated on build

/*
* Returns true if the keyexprs intersect, i.e. there exists at least one key which is contained in both of the sets defined by self and other.
*/
autocanonize(other: IntoKeyExpr): KeyExpr {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just wondering why autocanonize is a method of already created KeyExpr? This function should accept string and create key new expression. I.e. this is a constructor. It can be implemented like this:

class KeyExpr {
...
	public static autocanonize(s: string): KeyExpr | null {
		const key_expr = ??? call_wasm<String>(s, autocanonize);
                // we should be able to call wasm without `this`
		if key_expr.is_ok() { // not sure how to really check it to ok
                   return new KeyExpr(key_expr)
                } else {
                   return null; // TODO: it is the way how we fail in zenoh_ts? Need to check
                }
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants