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

Add assignString to AssignmentNodes #2980

Closed
wants to merge 1 commit into from
Closed

Conversation

webdeb
Copy link

@webdeb webdeb commented Jun 22, 2023

This PR replaces the default behaviour of AssignmentNode and FunctionAssignmentNode to use := as the Latex output for assignment String. It also forces the ConditionalNode to use := for the condition, trueExpr and falseExpr.

Additionally it uses the assignString in toHTML and toString methods.

In the usual cases for AssignmentNode and FunctionAssignmentNode the user can decide which assignString to use. Either by providing the token as "(a := 3)" or by changing the assignString programmatically. Like:

n = new AssignmentNode(...)
n.assignString = ':='

The default assignString is =, as the := is mostly used for definitions, like in the conditional node case. And most visitors are used to the simple = syntax.

#2979

return new AssignmentNode(new SymbolNode(name), value)
const assignmentNode = new AssignmentNode(new SymbolNode(name), value)
assignmentNode.assignString = assignString
return assignmentNode
Copy link
Author

Choose a reason for hiding this comment

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

@josdejong please let me know what you think about this, because it does not feel totally right to me as well doing it by assigning the assignString directly to the object. Maybe its better to provide it as param? but then some more checks in the constructor would be needed, because of node.index which is optional. Or as a separate chainable method, like

return new AssignmentNode(...)
  .setAssignString(assignString)

Again, all this is actually because of the := assignString for the toTex representation.

@webdeb
Copy link
Author

webdeb commented Jun 23, 2023

Found the commit were the assignment was changed to := 0868987 It was part of more a big refactoring. Unfortunately this part was not discussed at all. And I think the best one can do is to let the user decide, how the representation should look like.

@josdejong
Copy link
Owner

Thanks for working out this PR @webdeb .

It is an interesting idea to make the assigns string (= or :=) customizable. I'm in doubt though how big the need for this customization is, and whether it is worth building in a configurable solution for this case. I would like to postpone making this customizable until there is a clear need for it. You're the first that asks to change := to =. It can well be that everybody will be just happy when the assignment string is = and that's it.

There are workarounds for this BTW: with a simple regex you can replace := with = and vice versa. And you can also customize the output by providing a custom handler as described here: https://mathjs.org/docs/expressions/customization.html#custom-html-latex-and-string-output.

@webdeb
Copy link
Author

webdeb commented Jun 28, 2023

Thank you for looking into it!

It can well be that everybody will be just happy when the assignment string is = and that's it.

I agree, I just thought, that for some cases, you want this to be a real variable definition, like:

a := 2
c := 10

f(x) = a^x * b^b 

whatever.. also, wasn't sure if this was a strong requirement back then. So I thought a flexible solution would fit all needs, if required.

@webdeb
Copy link
Author

webdeb commented Jun 28, 2023

There are workarounds for this BTW: with a simple regex you can replace := with = and vice versa. And you can also customize the output by providing a custom handler as described here: https://mathjs.org/docs/expressions/customization.html#custom-html-latex-and-string-output.

Btw, before opening a new issue, can the handler be globally configured or would you suggest just monkey-patching the toTex method?

@josdejong josdejong mentioned this pull request Jul 5, 2023
8 tasks
@josdejong
Copy link
Owner

josdejong commented Jul 5, 2023

I've listed the change from := to = in the planned breaking changes: #3032

Btw, before opening a new issue, can the handler be globally configured or would you suggest just monkey-patching the toTex method?

You can always write a wrapper function customToTex(node) which applies the configuration and changes you want. You can probably also override the method math.Node.prototype.toTex though I'm not a big fan of that.

@josdejong
Copy link
Owner

I'll close this PR now since we will go for another solution (simply replacing := with =). Thanks for your inputs!

@josdejong josdejong closed this Jul 12, 2023
@josdejong josdejong mentioned this pull request Sep 20, 2023
6 tasks
@josdejong
Copy link
Owner

josdejong commented Oct 26, 2023

The assignment operator used in toTex is now changed from := to = in mathjs v12.0.0

@webdeb
Copy link
Author

webdeb commented Oct 26, 2023

@josdejong I guess, you mean the other way around ;)?

@josdejong
Copy link
Owner

josdejong commented Oct 26, 2023

😂 ha, sorry, you're right, I updated my comment.

@webdeb
Copy link
Author

webdeb commented Oct 26, 2023

Thank you!

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.

None yet

2 participants