-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add <formula-editor /> component #4
base: formula-editor
Are you sure you want to change the base?
Conversation
…-server Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
@@ -0,0 +1,62 @@ | |||
export class Stack<Type> { | |||
private _inner: Type[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inner can be named more descriptively
similarly for Queue
return this._inner.at(-1); | ||
} | ||
|
||
empty(): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty -> isEmpty
export enum Expectation { | ||
VARIABLE, | ||
OPERATOR, | ||
UNDEF, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UNDEF -> UNDEFINED
@@ -0,0 +1,127 @@ | |||
export class Recommender { | |||
private _trie: TrieNode; | |||
private _minSuggestionLen: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_minSuggestionLen -> _mininumSuggestionLength
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
…entation Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
if (!editor) return; | ||
|
||
this.parseInput(recommendation); | ||
this.currentCursorPosition = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this?
return false; | ||
} | ||
|
||
static getCaret = (element: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getCaret -> getCaretPosition
similarly for setCaret
this.currentCursorPosition = null; | ||
} | ||
|
||
parseInput(addRecommendation: string | null = null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addRecommendation -> recommendation
|
||
this._calculatedResult = calculatedResult ?? NaN; | ||
this._errorStr = | ||
calculatedResult == undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
condition doesn't seem intuitive
></suggestion-menu>` | ||
: html``} | ||
<div id="wysiwyg-err" class="${this._errorStr ?? "wysiwyg-no-err"}"> | ||
${this._errorStr ?? "No Errors"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can choose to show \n instead of "No Errors"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newlines are treated the same as spaces, so I am changing to no breaking space instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can consider to move these functions to helpers.ts
Signed-off-by: Ravi Dev Pandey <[email protected]>
tokens.forEach((token) => { | ||
// It is a number is either it's in the defined variables, or | ||
// it's a valid number literal. | ||
let isNumber = this.variables.has(token) || !Number.isNaN(Number(token)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about using a tokenType=number | operator | bracket | space
instead of having 4 such variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I was thinking of having an Enum for this. Seemed like a more structured approach to me.
} | ||
|
||
const tokens = formula | ||
.split(/([-+(),*^/:?\s])/g) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can have the delimiters defined as a const, to be used at multiple places
Signed-off-by: Ravi Dev Pandey <[email protected]>
Signed-off-by: Ravi Dev Pandey <[email protected]>
646a777
to
6f72838
Compare
Fixes #2
What is done ?
Possible bugs/Needs testing
-
and+
operators / Handling negative numbers.What remains ?