Fix for: CommunityStore/Tax/TaxRate.php:295 A non-numeric value encountered (2)

If a product is configured as not being shippable and you then try to delete the item from the basket modal, the following error is encountered:
Exception Occurred [snip] /packages/community_store/src/CommunityStore/Tax/TaxRate.php:295 A non-numeric value encountered (2)
Fix: /community_store/src/CommunityStore/Tax/TaxRate.php wrap the getFloat() call on line 287 in a floatval() as follows:
$shippingTotal = floatval(StorePrice::getFloat(StoreCalculator::getShippingTotal()));

Thank you for this one, I've tracked this down and pushed a fix for this (plus a bit of cleanup):
https://github.com/concrete5-community-store/community_store/commit/dc1628adb1414e938573a02bec52b1a14060938a


[Event Request] Codeunit 91 Purch.-Post (Yes No) - OnConfirmPostPurchaseHeaderIsOrder

Hi, would it be possible to get an event in codeunit 91, procedure ConfirmPost, like this :
    local procedure ConfirmPost(var PurchaseHeader: Record "Purchase Header"; DefaultOption: Integer): Boolean
    var
        ConfirmManagement: Codeunit "Confirm Management";
        Selection: Integer;
    begin
        if DefaultOption > 3 then
            DefaultOption := 3;
        if DefaultOption <= 0 then
            DefaultOption := 1;

        with PurchaseHeader do begin
            case "Document Type" of
                "Document Type"::Order:
                    begin
                        IsHandled := false;
                        OnConfirmPostPurchaseHeaderIsOrder(PurchaseHeader,IsHandled);
                        if not IsHandled then begin
                            Selection := StrMenu(ReceiveInvoiceQst, DefaultOption);
                            if Selection = 0 then
                                exit(false);
                            Receive := Selection in [1, 3];
                            Invoice := Selection in [2, 3];
                        end;
                    end;

    [IntegrationEvent(false, false)]
    local procedure OnConfirmPostPurchaseHeaderIsOrder(var PurchaseHeader: Record "Purchase Header" ; var IsHandled : Boolean)
    begin
    end;

?
Thanks

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. We will update this issue with information about availability. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.


Adding SubsetSplitter

Adding SubsetSplitter to nbs

Thanks!

Mmm studying this before exporting, but it's not a splitter since it does not take a list of items, but thins that are already split. Yet you have an assert for the sum of the sizes.
I rewrote it a bit to actually take splits from a list of items and adapted the test.


Fluent Icon Redesign & Enhancement

I have redesigned the XmlNotepad’s icon to align with the Fluent/WinUI3 Style, the latest design language for Windows apps. The new icon retains the original xml logo on the notepad icon, and supports proper icon displaying on high-resolution displays with 256x256 pixels. It also enhances the visual appeal and clarity of XmlNotepad on Windows platforms.

awesome, do you want any special mention in the about dialog for his?

awesome, do you want any special mention in the about dialog for his?

If you'd be so grateful, I'll ask.


Small spelling error

"report is always available is LMPs were discovered"
should be
"report is always available if LMPs were discovered"
In other words: is -> if

@TomAafloen Thanks for this. We'll search the docs and fix. Appreciate you taking the time to contribute to our community!

#please-close


Fix for issue #2535 "TypeError: this.text.indexOf is not a function"

"TypeError: this.text.indexOf is not a function" exception occurs for js/html5 targets if value consists of numbers only - so clearly cast to string.
Haxe code to reproduce:
smth. like this
private function setValue(strNumber:String):Void {
	textField.text = strNumber;
}

The type of the setter is already String, so if you are managing to pass in a different type, it should be considered a bug in your code. Here's the function signature. The compiler will not allow any other value here, unless it's typed as Dynamic.
private function set_text(value:String):String

It's "dangerous" to use Dynamic, and you need to be extra careful when you use it. In this case, the fix you need somewhere in your code is to convert the value to a string. Std.string() is a good way to do that.
var strNumber:Dynamic = 123.4;
textField.text = Std.string(strNumber);


nuvoton: battery-backed ram

Add bindings and entry for the battery-backed ram in the nuvoton
npcx7m6fb. This commit is an upstream port of
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform/ec/zephyr/dts/bindings/cros_bbram/nuvoton,npcx-cros-bbram.yaml;l=1;drc=e9af813c36b7b411bf2a01cbc1b09d5fdec49b8a
Along with a bit of cleaning up.
Signed-off-by: Yuval Peress<EMAIL_ADDRESS>

Any updates here?

Yes, sorry, I've been ill and it escaped me. I had things mostly working when I left it, I'll try to get this up and running in the next few days.

check_compliance.py seems buggy, it's complaining about undefined Kconfigs which I did define in this change and one that is unrelated to this change.

@galak any thoughts on this? Can it merge?

Added an emulator and tests along with it.

Added an emulator and tests along with it.

+1 for adding the emulator. We need more of these :-)


New and improved IK in Skeleton2D

This pull request is the 2D work for the new and improved IK in Godot, as part of my Google Summer of Code proposal. This PR includes behind the scenes changes, like changes to Skeleton2D and Bone2D, as well as the IK code itself. You can find the 3D equivalent for this PR here: #39353

Note: See this branch for the verbose commit log for commits 1-8: https://github.com/TwistedTwigleg/godot/tree/GSOC_2020_Working_Branch_2D_IK_VERBOSE_ARCHIVE
The first 8 commits are squashed changes from the verbose log, bringing the commit count from 34 to just 8. Commits after commit 8 are new changes added on top of the changes in the verbose archive linked above.

Below is a list of everything that has been added, removed, or otherwise changed. The list is in no particular order.

Changes

Added a SkeletonModifier2D class

Works just like the SkeletonModification3D class, just for 2D. It extends Resource, can be saved to a file, will be used by IK, is extendable via GDScript, and all modifications should be fully compatible with each other for mix-and-matching.
Has a function for clamping angles, as it is needed by almost every (if not every) SkeletonModification2D that extends it.
Has a property that determines whether the modification will run in either _process or _physics_process! This is a per modification property.

You can now mix-and-match physics aware modifications (only Jiggle right now) with non-physics aware modifications!

Has an editor-only function for drawing gizmos in the Godot editor using the Skeleton2D node's draw functions!

Extremely helpful for showing things like angle constraints!
The gizmo can be turned off via the modification settings and modifications that are not enabled will not be drawn. Modifications are drawn in the editor regardless if they are executing so things like angle constraints can be adjusted without IK running.
The gizmo drawing system for SkeletonModification2D resources is also exposed to GDScript, so users can make custom drawn gizmos if desired. See test 10 in the 2D test project for a simple working implementation.

|

Added a SkeletonModifierStack2D class

Works just like the 3D version: holds all of the SkeletonModification2D resources that are going to be applied to a Skeleton2D. Has settings for enabling/disabling all modifications, can set the interpolation strength, and can be saved to a file.
All modifications work with animations via the AnimationPlayer node, at least in my testing.
Modifications are now unique when duplicated, so they work with instanced/duplicated nodes/scenes.

|

Added SkeletonModification2DLookAt

Rotates a Bone2D in the Skeleton to look at the target. Simple, but can be very helpful.
Has an option to apply additional rotation, allowing for applying an offset.
Has fully working angle constraints that can be applied in both local-space or global-space. Angle constraints can also be inverted.
Has a fully functional editor-only gizmo that is drawn in the editor to show angle constraints. Can be disabled via a property.

|

Added SkeletonModification2DCCDIK

Uses CCDIK to solve a chain of bones, just like the algorithm in 3D.
Has full angle constraint support, including with inverted constraints. Like with LookAt, angle constraints can be enforced in either global-space or local-space.
Can rotate from the tip like normal CCDIK, or from the joint. This option is configurable on a per-joint basis.
Has a fully functional editor-only gizmo that is drawn in the editor to show angle constraints. Can be disabled on the entire modification, or on a per-joint basis.

|

Added SkeletonModification2DFABRIK

Uses FABRIK in 2D to solve the bone chain. Should work just like the current 2D IK solver for Skeleton2D, but as a SkeletonModification2D. Works in both the editor and in Godot projects.
Supports magnet positions, allowing for a measure of control over how the joints bend.
Has full angle constraint support that is taken into account when solving. Right now constraints only work in local-space (global-space support has not been added yet) Edit: angle constraints were not working. See comments below!
The final bone in the chain supports using the target's rotation, similar to the Basis override in SkeletonIK. It even works with angle constraints!
Has a fully functional editor-only gizmo that is drawn in the editor to show angle constraints. Can be disabled on the entire modification, or on a per-joint basis.

|

Added SkeletonModification2DJiggle

Rotates bones in a chain using jiggle physics. Great for hair, rope, edges of clothing, etc. Works just like the 3D version, but in 2D.
Has all the same options as the 3D version: stiffness, mass, damping, gravity, etc.
Has optional collider detection support! This is possible by running the modification stack in _physics_process mode. Uses raycasts to detect colliders.

It only applies to the motion created by the jiggle modifier. The jiggle modifier will not notice if a collider moves into it, but it will no longer move itself into colliders.
Options have been added to configure this functionality.
It actually works really well! It did require some modifications, but it also gives me a  better idea of how to handle physics with modifications. This also should be doable in 3D.

|

Added SkeletonModification2DTwoBoneIK

Works just like the 3D version, but in 2D. Uses the Bone2D node to get the lengths required, making it easier to get setup and running.
Has support for angle constraints, though these constraints are not taken into account by the algorithm. Removed!

I'm not sure if there is a way to make the algorithm take the angle constraints into account when solving, making me unsure if angle constraints should be kept or not. The code is relatively easy to add or remove, so I left it in for now.
Removed because the constraints are not taken into account by the solver, which would be confusing to users as why it doesn't seem to "work". It also makes the code for TwoBoneIK smaller, as all of the properties can be removed. Maybe in the future I'll take another look at implementing angle constraints, but for now, I'm just going to leave it without.

Has a boolean, called "flip bend direction", that changes whether the TwoBoneIK modification will push the bones inward or outward when solving.
Has properties for defining the minimum and maximum distances the TwoBoneIK modification can take into account, allowing the user to control how far the solver can extend and contract when solving.
Has a fully functional editor-only gizmo that is drawn in the editor to show the bend direction. Can be disabled via the modification properties.

|

Added PhysicalBone2D

The new node extends RigidBody2D, allowing for full physics support and use of the RigidBody2D API. You can apply forces, set it to sleep, change the mode, etc.

Because it extends RigidBody2D, anything added, fixed, or modified in RigidBody2D will also benefit the PhysicalBone2D node automatically!

PhysicalBone2D has to be a child of a Skeleton2D node or another PhysicalBone2D node to function. This is similar to PhysicalBone3D in that respect.
PhysicalBone2D takes a Bone2D index (or NodePath to a Bone2D) and will automatically position itself accordingly.
Currently, PhysicalBone2D nodes that are children to PhysicalBone2D nodes recommend having a Joint2D node as a child to keep the joint chain intact.

The PhysicalBone2D has settings to auto configure the Joint2D settings. Right now this is limited to setting Node A and Node B in the joint, as well as setting the joint's position.
This may change in the future though!

Informs the user of any configuration issues via the get_configuration_warning function.
Can be applied to Bone2D nodes via a modification.
Supports Joint2D nodes and any constraints these nodes apply.
The creation of this node is to prototype how I may adjust PhysicalBone3D, making it more independent and usable with the new skeleton modification system.
Has a property to follow the Bone2D node when simulating, which is necessary for a PhysicalBone2D to be used as a root in a physics chain.

|

Added a SkeletonModification2DPhysicalBones

This modifier handles applying the transform from the PhysicalBone2D nodes to the bones and the skeleton.
Has functions that mirror the PhysicalBone2D API seen in Skeleton3D, but instead these functions are for PhysicalBone2D.
Exposes a function (and editor-only-property) that allows for fetching all PhysicalBone2D nodes in the Skeleton.
(Modification names are kinda long. Maybe I should shorten them? Hmm...)

|

Added a SkeletonModification2DStackHolder

This is a simple modification that allows users to use more than one modification stack with a single Skeleton2D node. Can be helpful for organization and modification reuse.
Known limitation: A stack holder modification can only execution modifications in the stack it's holding if the modification execution mode lines up with the stack holder modification's mode.

|

Made various changes to Skeleton2D:

Skeleton2D now holds a SkeletonModificationStack2D.
Added functions for setting a local_pose_override to Skeleton2D. The local_pose_override works just like it does for Skeleton3D, and can even be interpolated! It does require using RenderingServer to apply the override, but it works pretty well with only minimal changes required.
No longer uses a RenderingServer trick! Now it directly sets the transform of the Bone2D node.
Supports executing the modification stack in either _process or _physics_process, based on the information in the SkeletonModificationStack2D.
When modifications are running, the Bone2D nodes being modified change to the editor IK color.

|

Made various changes to Bone2D:

Bone2D now has a property called length, which is the bone's current length. default_length has been removed and its functions deprecated. length can be set in the editor like normal, so there's no loss of functionality.
Bone2D now has a property called bone_angle. bone_angle stores the angle that the bone has, which can be different than the transform's rotation.

For example, if you have a Bone2D making a right triangle. Since the Bone2D nodes are not rotated, but the bones themselves are (since bone rotation is determined by child Bone2D nodes), the rotation of the transform is not accurate enough for IK. bone_angle solves this issue by storing the angle of the bone. It is kinda hard to explain, but without it, all bone chains would have to be straight.

Bone2D now has a boolean for auto calculating the length and angle of the Bone2D, which is enabled by default. The autocalculation will use the first Bone2D child node it finds to calculate the length and angle. If there are no Bone2D children, it will just take the Bone2D's transform rotation instead and will print a warning.
Functions have been added for the various additions and properties exposed as required.
Bone2D internally caches it's transform when it is not being modified by a Skeleton modification. This is needed for proper pose interpolation between the bone's transform and the modification result.
Bone2D now handles its own editor gizmo via the _draw function.

The gizmo works exactly like before: It uses the same shape, color, etc. The only things that have changed is that it is not anti-aliased by default (due to project settings not having AA by default) and it no longer scales with the zoom level of the editor.
This makes the Bone2D gizmo in the proper position when overridden by modifications. This theoretically is possible in CanvasItemEditor, but I couldn't get it working and I even if I could, I fear the code would be rely on hacks and difficult to manage.
The gizmo now takes the bone angle into account, removing the need for using a Bone2D node just to have the Bone2D gizmo drawing the bone with the correct rotation.
The gizmo code only executes in the editor and uses #ifdef TOOLS_ENABLED, so no performance is lost on exported projects.
In the editor, a boolean property is exposed to allow for showing/hiding the editor gizmo. This option only is available in the Godot editor.
The editor colors for Bone2D gizmos have been adjusted so they are more transparent by default, making them easier to use with Sprite2D and Polygon2D nodes.

|

CanvasItemEditor changes:

The code for drawing Bone2D gizmos have been removed from CanvasItemEditor.
The code for 2D IK in CanvasItemEditor has been removed. Now Bone2D is the only bone (or bone-like) thing in 2D.
A few of the options in the Bone dropdown have been removed, like Clear Bones and the IK related options, as they are no longer needed or usable.
The Show Bones option now toggles the editor gizmo visibility of any selected Bone2D nodes and any child Bone2D nodes for those selected.
The Create Custom Bone2D(s) from Node(s) option will make Bone2D nodes using the selected Node2D nodes, and will re-parent the selected nodes to the created Bone2D nodes, making the creation process much easier and similar to how the Create Custom Bone(s) from Node(s) option worked.
The editor_selection variable has been changed to a public variable in CanvasItemEditor, as otherwise there is no great way for Bone2D nodes to know if they have been selected.
Its possible these changes are comparability breaking with Godot 3 projects that heavily rely on the old 2D IK system. I don't think there will be any issues, since old 2D IK saves the positions and doesn't work during runtime, but I have not tested. I need to find a project that uses the old IK system to double check.

|

Added a looking_at function to Transform2D.

This function is now exposed to GDScript and documented.

|

Added two new notifications to the Node class: NOTIFICATION_EDITOR_PRE_SAVE and NOTIFICATION_EDITOR_POST_SAVE.

These notifications are sent to all nodes in the scene right before (pre) and right after (post) saving the scene in the Godot editor.
This was needed to correctly reset the Bone2D nodes to their cache positions when saving the scene, so scenes saved with modifications running do not save the result of the modification over the position of the Bone2D node prior to the modification running.
These notifications will be extremely helpful for Godot users writing plugins, as now there is a way to detect when the editor is going to save and has saved, so now any preparations prior to saving can be properly performed. This was something that I found missing when I was writing Godot plugins, and something I have seen as a common question by those making plugins for Godot. (To be fair though, it does have a limited appeal.)
The new notifications are only called in the Godot editor, and the class reference has been updated to include the new notifications.

|

Added documentation for all new additions and any changes made.

|

Probably other little changes I have forgot!  I will try to keep this list as accurate as possible though.

Todo:
Nothing for now!
Known bugs/errors/problems:

None of the IK modifiers work when the Skeleton2D is scaled to a negative number on the X or Y axis.

Edit: Maybe this is not a bug? I did some looking around, and it appears a lot of Godot nodes don't work with negative scales (based on my Google searches). It seems to flip a character, the visual nodes should be flipped rather than using a negative scale. This may mean that the modifications not working with negative scales is normal/okay?

Known limitation: FABRIK and TwoBoneIK only work with uniform scales.

This is due to their reliance on bone length and how they handle scaled Bone2D nodes. Ideally this should be fixed by using a more universal solution, but I have yet to find one (as of when this was written).

Setting bones before setting the target causes some modifications not to run. Can not reproduce anymore, so I'm going to assume this bug is fixed.
Saving a scene with SkeletonModification2D resources attached to a Skeleton2D cause them to no longer work in the editor, though functionality returns upon reloading the scene. This is a known issue that also occurs in the 3D PR. Fixed! Used the same fix that I found in the 3D IK PR.

The test project I am using can be found here.
Disclaimer: Some of the work done in this PR was part of the Google Summer of Code 2020 program!

Alright, I think this PR is ready for review now! :tada:

Just an FYI to anyone looking - I need to update the 2D IK PR to the latest version of master to fix a merge conflict that has arisen. I was going to wait until the end of the GSOC, but because of the merge conflict, the CI tests are not running. Thankfully it appears everything is compatible going forward, with only PhysicalBone2D having some issues in the latest version of master. I'll rebase this PR soon and will update this comment once the PR is fully rebased.

Updated the PR to use the latest version of master to fix the PhyiscalBone2D issue. That should, hopefully, be the last force push required for this PR (fingers crossed).

@JFonS, @akien-mga - I think this PR is ready to go now.
Everything is implemented and all of the bugs @pwab and I have found should be fixed (with the exception of #42134).
Also, is there anything I need to do to make the CI work with the tests=yes builds? I looked at the error logs provided, but I'm not sure if it is anything I touched that is causing the issue.

Also, is there anything I need to do to make the CI work with the tests=yes builds? I looked at the error logs provided, but I'm not sure if it is anything I touched that is causing the issue.

tests/test_class_db.h:587: ERROR: CHECK_FALSE( bad_reference_hint ) is NOT correct!
  values: CHECK_FALSE( true )
  logged: "Return type is reference but hint is not 'PROPERTY_HINT_RESOURCE_TYPE'. Are you returning a reference type by pointer? Method: 'SkeletonModification2D.get_modification_stack'."

You can run the tests locally by building with tests=yes and then running godot --test.
tests/test_class_db.h ensures that ClassDB bindings are valid, and it seems like your binding for SkeletonModification2D.get_modification_stack is problematic.

You can run the tests locally by building with tests=yes and then running godot --test.
tests/test_class_db.h ensures that ClassDB bindings are valid, and it seems like your binding for SkeletonModification2D.get_modification_stack is problematic.

Good to know! I totally missed the error on the get_modification_stack function.
It should be fixed now, as now the function returns Ref<SkeletonModificationStack2D> rather than a raw pointer, which made the tests run successfully locally.

Added a gizmo for showing the minimum and maximum for two bone IK and rebased with master :+1:

Rebased with the master branch to fix merge conflicts!

@TwistedTwigleg Would be nice if you could apply the changes that we agree on, on the 3D part too; so the 3D part review will be  faster.

@TwistedTwigleg Would be nice if you could apply the changes that we agree on, on the 3D part too; so the 3D part review will be  faster.

Thanks for the review @AndreaCatania!
I’ll make the changes once I can. I’ll also make the changes to the 3D side as well once I’ve changed the 2D stuff.
It may take till the weekend, but I’ll see if I can chip away at it over the week. Regardless, I’ll let you know once the changes have been made to both PRs.

Thanks for the review @AndreaCatania!
I’ll make the changes once I can. I’ll also make the changes to the 3D side as well once I’ve changed the 2D stuff.
It may take till the weekend, but I’ll see if I can chip away at it over the week. Regardless, I’ll let you know once the changes have been made to both PRs.

Ty, consider that I'll spread the review across many days.

Ty, consider that I'll spread the review across many days.

Alright, I made the changes! I had a bit of time and since they were all in the same file, it wasn't too hard to quickly go through them.
Do you want me to push the commit or wait? Also, should I wait on the rebase so the review is easier?
(I'll try to update the 3D IK PR after commits, so I can use the commit difference log to tell what to update in the 3D PR)

Alright, I made the changes! I had a bit of time and since they were all in the same file, it wasn't too hard to quickly go through them.
Do you want me to push the commit or wait? Also, should I wait on the rebase so the review is easier?
(I'll try to update the 3D IK PR after commits, so I can use the commit difference log to tell what to update in the 3D PR)

Yes, do the rebase and commit it, It's fine.

Yes, do the rebase and commit it, It's fine.

Yes, do the rebase and commit it, It's fine.

Awesome, will do as soon as I have the time!

The PR has been rebased with the latest changes and the commits added. Sorry about the wait and thanks again for the help!

The PR has been rebased with the latest changes and the commits added. Sorry about the wait and thanks again for the help!

Huge thanks for the review, I'll start making the adjustments when I have the time :+1:

Huge thanks for the review, I'll start making the adjustments when I have the time :+1:

Alright, I made modifications based on the review feedback. I didn't look at the gizmo stuff, but everything else was adjusted according to the comments left :+1:

Alright, I made modifications based on the review feedback. I didn't look at the gizmo stuff, but everything else was adjusted according to the comments left :+1:

I changed the use of LocalVector to Vector again, so the CI would work. I'm not sure what about LocalVector was causing it not work, especially since it compiled fine and without error on my end, but it seems that using Vector fixes the issue. I did remove the modification_count variable though, since it was unused and not needed regardless of what data type is used.
Now the CI should hopefully work!

Also, I tired converting the Gizmo code back into canvas_item_editor_plugin after looking through the Editor Plugin code I was linked to for the Gizmos. I was initially looking to make a editor plugin, like those in 3D I was pointed to, but it seems there is no 2D equivalent to the 3D plugin system except the code in canvas_item_editor_plugin, which is what I attempted to do.
However, after several hours of attempting and debugging, I couldn't get it working. The issue was that I couldn't get anything to draw, even after validating that the data I was using was correct and the functions were being called. I'm not sure what the issue was, but nothing seemed to work. I also tried using code that was almost identical to the code I removed from canvas_item_editor_plugin but it didn't work either.
I may be misunderstanding how the code in canvas_item_editor_plugin works, which is possible, its 6,000+ lines and the code for drawing is scattered throughout the file, or that I simply was missing something. Regardless of the reason, I cannot seem to get it working and unfortunately do not have the time currently to carefully debug the issue and create a new gizmo solution for Bone2D nodes.

So, I just removed angle constraints from 2D FABRIK, as they were not working. This was my bad, I didn't test enough and so when I saw the first bone in the FABRIK chain was working correctly, I assumed the entire thing worked. This turns out not to be the case. The constraint system did not work for any bones beyond the first one, and it doesn't even work on the first one if the first bone has a parent bone that is rotated, making it effectively useless in real-world use. I should have tested more during the GSoC period, so that is my fault.
I only discovered that 2D FABRIK constraints were not working when I tried to adapt the GSoC method of angle constraints in FABRIK to Twistd IK 2 and found it was not working. I then cross-verified by checking the GSoC method in this PR, confirming that it, in fact, doesn't work. I haven't found a method for Twisted IK 2 either, but if I find a way to make constrained FABRIK in 2D that works, I will document the method publicly so it can potentially be used in this system as well.
(You can kinda get something similar by using FABRIK and then running CCDIK with the angle constraints you want, so that's something I guess)
Also, I fixed one of the issues that the static checking pass was having. I need to rebase with master, which I will do momentary and will edit this comment once done.

Thanks for the review @pouleyKetchoupp! I made some changes based on your feedback. The biggest change is that the modifications are now split into their own files for better readability.

To answer some of the questions brought up:

Using _set and _get is only for implementing custom properties which can't use the standard bindings.
... It would also make things exposed to script more consistent. For instance in your implementation, if I'm not mistaken, calling set_constraint_angle_min(angle) from gdscript would take a rad value, while constraint_angle_min = angle is expecting a value in degrees.

Since its already written and working, I don't see the need to use the macros currently. This can always be changed in a future PR, since it's just a minor change.
As far as the exposed functions go, they are all in radians. Using constraint_angle_min as an example, if you call set_constraint_angle_min it will expect a value in radians and same if you set it directly. The only place where it expects degrees is in _set and _get, as the documentation did (and I think still does) mention that all rotations shown in the editor are to be in degrees but the functions themselves should be in radians.

I've tested on the linked project and things seem to work correctly (after solving conflicts on latest master). I got some warnings and errors which I didn't understand though. I'm wondering if they are supposed to occur, and if they could have more details about what to do to fix them if it's a problem in the setup/script:

I've changed the error for updating the target cache to a warning, since it's not really anything that the user should concern themselves with in most cases. Both of the issues you mentioned occur because the IK modification doesn't have the data cached, and this can be because it cannot get the data from the Skeleton2D/Bone2D, or it could simply be that the cache is out of date (which happens on the first run).
The reason its printed at all is because it helps diagnose the errors if the issue is that it cannot get the data, but I can see how this can also be confusing. Again, in most cases the caches updating are not an issue, though there are situations where these warnings could help.
I'm not sure on what would be the best solution currently. I did remove the use of the custom error printing function and used the ERR_PRINT_ONCE and WARNING_PRINT_ONCE macros instead though, so at the very least these messages will only be printed once.

Another thing is in one of the test scenes (Test_10), the script tries to access some unexposed functions:
...
Any idea if the problem is from the script or if something went wrong during a rebase?

This just affects the GDScript code. The GDScript code was written months ago right when GDScript 2.0 was merged, so the code is old and references functions that have since changed. I think the looking_at function was also removed from this PR, if I recall correctly.
Regardless, it shouldn't affect this PR nor the 2D IK in any way.

About the gizmo code, it needs to be moved to the editor code at some point, but since you had some trouble with the implementation it can be postponed so the rest can be merged.
Maybe you can just add something like that in Bone2D::_notification to keep track of this issue:

Added the warning :+1:

Thanks again for the review!

Also, something I just wanted to note in general:
My time and energy is limited due to having a full-time job, administrating on the Godot community forums (full time), working on my own projects and plugins, and other personal activities. I understand that these things take time, but its been months since I've worked on these PRs and the code is becoming increasingly unfamiliar. Additionally, movement on this PR seems to be rather slow, similar to the 3D Maze Demo I made and I worry that it will reach a similar fate after months of going back and forth.
If this PR will be merged in a timely manner, than I will continue to maintain this PR and the 3D IK PR. Once merged it will be up to the community to maintain. I will be happy to help advise and contribute small changes infrequently, but I cannot take the responsibility of maintaining and overseeing the 2D and 3D IK code, as I simply do not have the bandwidth.
Thanks for your understanding!
(I will update the 3D IK PR and make changes using the feedback from this PR, as I said I would. I am just waiting for this PR to be merged first)

@TwistedTwigleg For info, there are code format errors to fix in skeleton_2d.cpp, static checks currently fail.

@TwistedTwigleg For info, there are code format errors to fix in skeleton_2d.cpp, static checks currently fail.

Fixed

Thanks for the reviews guys! I’ll look at making the changes soon

Alright, I made the changes based on the review feedback!
Thanks for all the help! :)

@AndreaCatania - for some reason its not letting me reply directly to the code review, so I'll have to reply here: For the new-line, it is not a typo. I broke it down into two lines because it was really long and didn't fit my monitor. I can change it to a single line if that would be better though.

@AndreaCatania - for some reason its not letting me reply directly to the code review, so I'll have to reply here: For the new-line, it is not a typo. I broke it down into two lines because it was really long and didn't fit my monitor. I can change it to a single line if that would be better though.

That's fine for me, you can leave it as it; personally when the line is too long I like break it in multiple lines (one line per variable).

Will need a rebase to solve merge conflicts, and to make the commit history easier to read once merged. Each commit has a very detailed description but commits named as "Changes:" will simply appear as "Changes:" in the changelog and that's not a useful description for a changelog reader.
I'm not sure what's the best way to improve the commit history, 30 commits is too much (imagine doing a git bisect and having to compile several steps in the middle of this big refactoring while investigating something not IK related at all), but I don't necessarily advocate for squashing it all into one mega-commit. Some in-between would be nice, but I'm aware that it's work to meld some of the commits together into bigger yet self-contained changesets.

Thanks for the review @akien-mga, I'll look at making the adjustments and rebasing with the master branch this weekend :+1:
(I'll also reply to any questions on the review feedback then as well, as I go through them)
As for the commit number and merging: I think that either merging everything into a single commit or making a new PR/Branch with a reduce commit log is probably the easiest of the options available. Of the two I'd prefer to merge it as a single commit, but I can see the downsides and why it may not be desirable.
If we go down the making a new PR route, I agree that it should be done after all changes are made and everyone agrees that its good to go. This PR has already had its commits squashed using a similar method before (see linked verbose branch above) and while I do not mind doing it again, I would prefer this be the last time doing such a big rebase/commit-squish for the material in this PR.
Also, assuming we want to do the new PR route, can you give me some examples of the commit format you would prefer? Would just adding a generic, short message of what has been added and then "Changes:" with the changes suffice or would you prefer something else? Please let me know how you would prefer it and do my best to write the commit messages accordingly.

A new PR with a single squashed commit seems good to me. It's not uncommon for us to merge fairly big changes as a single commit.
For the commit log, I have an example here: https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind
So the main structure is that the first line is the title of the commit, and it should give a summary of what the commit is about (typically can be the same as the PR title, so "New and improved IK in Skeleton2D" might be fine).
The rest is the body of the comment and is more free-form, you can format it as you like. A list of changes and/or a high level summary would both be fine with me. It's information both for potential users of the feature, and for future contributors who want to know what the commit addressed before digging into the details of the code changes.

Alright, everything has been rebased and the review feedback taken into account. I messed up one part of the rebase a little bit, but since this PR isn't going to be merged and a new, single commit PR will be instead, I figured it was fine to leave it.
Once everyone is ready and everything here is good to go, I'll make a single commit PR with these changes.
Thanks again for the reviews everyone! Also, thanks for the link and example on the commit message @akien-mga, I will structure the commit on the new PR accordingly.

Here's the list remaining issues that we need to keep track of:

Just to clarify, that's the list of issues we want to keep track of once this has been merged. They don't need to be addressed in this PR, as discussed previously.

Thanks for the review, I will try to address the remaining code style feedback issues this weekend.

Alright, the syntax changes have been made and now every file should be good to go. If everyone is good with how everything in this PR, I'll make a new PR with the commits squashed into one.

I'm not sure why the CI is failing, all I did was make the change suggested and it compiles fine on my machine. Regardless, I made the squished PR here: #47872

@TwistedTwigleg May I close this then?

Let's close this PR since #47872 was opened instead.


Detect unused {{ vars }} and whine loudly.

This makes sure any  {{variable}} referenced in the spec RST files actually map to a corresponding def render_variable(): section. Previous behaviour was to replace the template variable with an empty string (helpful, Jinja, helpful!)

LGTM


ADMIN-348 wrap apollo wrapper component over equipment campaign page to ensure the page is not loaded until data is present

I wrapped the apollo wrapper component over fellowship and constituency equipment campaign page to ensure the page is not loaded until data is present
Description

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Screenshots/Videos (if appropriate):

https://user-images.githubusercontent.com/36607706/192168620-194d234f-2609-4f9e-be04-aeea3979daf0.mov

I don't understand the video you've sent
You should have started recording from the time when you click to go to the next page


GH1034 Migrate old tests in _json.py to new check(assert(...

[x] Closes #1034
[x] Tests added: Please use assert_type() to assert the type of any return value

Simple test migration to the new check(assert_type fashion, no logic change.

Thanks for doing this clean up. The tests are ones we migrated from another repo when we started this project.
In lines 712 to 722, I think you can convert that to use the check(assert_type(...)) pattern.
In lines 923-924, there is this:
    s: Series
    s = DF.iloc[:, 0]

I think we can leave out the s: Series part because with s = DF.iloc[:, 0], I think the type inference will infer that it is a Series.

It is inferred as a Series so I was able to remove it.
For the first one, I addressed those, I missed those since I just addressed when we defined a variable and did not use it anywhere else, if you see any more, feel free to point me there.

if you see any more, feel free to point me there.

I did a search for : and didn't see anything else, so we're in good shape.  Thanks for doing this.  We have to clean up some of the other tests as well, but that's not as important as getting coverage for 2.x features, etc.


gcc leaves excessive address computations behind in dataflow paths

Run:
build/test/tt_metal/perf_microbenchmark/dispatch/test_bw_and_latency -m 2 -i 1
Look at the generated .elf file, see:
    3bd0:       14946c63                bltu    s0, s1, 3d28 <.L9>
    3bd4:       ffb207b7                lui     a5, 0xffb20
    3bd8:       2087a703                lw      a4, 520(a5) # ffb20208 <__global_pointer$+0x1fa18>
    3bdc:       ffb006b7                lui     a3, 0xffb00
    3be0:       16e6a223                sw      a4, 356(a3) # ffb00164 <__global_pointer$+0xfffff974>
    3be4:       2287a603                lw      a2, 552(a5)
    3be8:       ffb00737                lui     a4, 0xffb00
    3bec:       14c72e23                sw      a2, 348(a4) # ffb0015c <__global_pointer$+0xfffff96c>
    3bf0:       2047a603                lw      a2, 516(a5)
    3bf4:       ffb00737                lui     a4, 0xffb00
    3bf8:       14c72a23                sw      a2, 340(a4) # ffb00154 <__global_pointer$+0xfffff964>
    3bfc:       2007a603                lw      a2, 512(a5)
    3c00:       ffb00737                lui     a4, 0xffb00
    3c04:       16c72c23                sw      a2, 376(a4) # ffb00178 <__global_pointer$+0xfffff988>
    3c08:       22c7a703                lw      a4, 556(a5)
    3c0c:       ffb007b7                lui     a5, 0xffb00
    3c10:       14e7a623                sw      a4, 332(a5) # ffb0014c <__global_pointer$+0xfffff95c>
    3c14:       ffb487b7                lui     a5, 0xffb48
    3c18:       0107a583                lw      a1, 16(a5) # ffb48010 <__global_pointer$+0x47820>
    3c1c:       ffb007b7                lui     a5, 0xffb00
    3c20:       18478713                addi    a4, a5, 388 # ffb00184 <__global_pointer$+0xfffff994>
    3c24:       00c75503                lhu     a0, 12(a4)
    3c28:       16468693                addi    a3, a3, 356

Possibly related to compiler not optimizing out address calculations for loads/stores through volatile pointers
Another sequene from cq_dispatch:
ffc0054c:       ffb00b37                lui     s6, 0xffb00
ffc00550:       ffb00bb7                lui     s7, 0xffb00
ffc00554:       ffb00d37                lui     s10, 0xffb00
ffc00558:       ffb00cb7                lui     s9, 0xffb00

Yeah, I noticed some similarly suspicious address generation working on the crt code bloat.

These are all accesses to things like noc_reads_num_issued and the like.  Those are all global variables and the compiler has no idea that they're placed close together[*]
Fortunately these symbols are all withing gp range, if that was enabled, so that's what I'm working on.  It seems to reduce kernels by 4%-10%.
A more general technique would be defining a structure:
// in header
extern struct NOC_Counts { uint32_t reads, writes, atomic ... } noc[2];
// exactly once
NOC_Counts noc[2];
then the compiler will know these are close together.  One could use macro tricks to hide such a reorganization, but that's probably unwarranted.
[*] well I guess LTO could figure out something there, but it doesn't.

These are all accesses to things like noc_reads_num_issued and the like. Those are all global variables and the compiler has no idea that they're placed close together[*]
Fortunately these symbols are all withing gp range, if that was enabled, so that's what I'm working on. It seems to reduce kernels by 4%-10%.
A more general technique would be defining a structure:
// in header extern struct NOC_Counts { uint32_t reads, writes, atomic ... } noc[2];
// exactly once NOC_Counts noc[2];
then the compiler will know these are close together. One could use macro tricks to hide such a reorganization, but that's probably unwarranted.
[*] well I guess LTO could figure out something there, but it doesn't.

4% - 10% may translate to perf improvement on DM kernels?

done


cmake: media library dependency fixed to avoid possible linking error

hdzero-goggle/toolchain/bin/../lib/gcc/arm-openwrt-linux-muslgnueabi/6.4.1/../../../../arm-openwrt-linux-muslgnueabi/bin/ld: cannot find -lmedia
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/HDZGOGGLE.dir/build.make:1404: HDZGOGGLE] Error 1
make[1]: *** [CMakeFiles/Makefile2:78: CMakeFiles/HDZGOGGLE.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

+ whitespace cleanups

can restrict this one to the dependency change? lets take care of the formating once the dust has settled a bit.

can we restrict this one to the dependency change? lets take care of the formating once the dust has settled a bit.

good idea :)
I just pushed a new commit

lets see that we can get https://github.com/hd-zero/hdzero-goggle/pull/7 adjusted & merged,
then rebase and merge this one.


deprecate most config attributes

The Flask app object has some "config attributes", properties that are shortcuts to getting/setting certain app.config keys. There doesn't seem to be much consistency to which keys are exposed as attributes, and most of them don't seem useful as attributes.

debug, testing, and secret_key These should stay for now, they are shortcuts for common enough situations.
session_cookie_name It's not clear why this is an attribute. None of the other session cookie options are exposed as attributes, and it seems much more common to want to change those.
permanent_session_lifetime I think this is an attribute only so that it doesn't have to be converted from int to timedelta in two places. However, just like the name, this doesn't seem like something that belongs as an attribute more than the other options.
send_file_max_age_default Again this does a conversion from int to timedelta. But it's only used in one place, get_send_file_max_age.
use_x_sendfile This only works when behind a correctly configured Apache server, not any other server setup. It's not clear why this should be an attribute, it's a very niche case.
env This is going away with #4714
propagate_exceptions is only used in handle_exception, and is a very internal behavior. This property can't be set, but it does have some logic to fall back to testing or debug.
templates_auto_reload I added this as part of making setting debug reset the property on the Jinja env. However, setting this property itself doesn't do that. There's no particular reason to expose this as an attribute.

permanent_session_lifetime is a helper for custom session interfaces to not need to do the int/timedelta conversion, so I'll leave that.


Dates in nonstandard formats

When parsing in metadata_samples.json, I've noticed some of the date are not parseable by datetime.date, e.g.:
  "ERR1224351": {
    "country": "Thailand",
    "date": "Summer 2013",
    "location": "Bankok",
    "reads": 95437411
  },

It would be nice if these were standardized in some way, either as valid dates or in some other way I can parse and connect to the prevalence data.

Not sure the best way to represent these.  I think the problem was that some were from June and some were from August, and I couldn't figure out from their metadata which were which. I could split the difference and say "2013-07", or just "2013"?

Not sure if it's worth it, but you could introduce a "season" field if this happens often enough and make date a year.

Not sure if it's worth it, but you could introduce a "season" field if this happens often enough and make date a year.

Unless/until a 'season' field seems obviously useful, you might follow this suggestion of doing whatever you need to do to keep date as a well defined format, which here might be setting it to the year, and using a general-purpose 'notes' field to record that the sample is from June or August


Edit mode causes viewstate to be lost

Description of bug
This is a known issue but never got fixed and continue to affect DNN 9.6.1. See old discussion from DNN-10523 - Webform postback in Edit mode loosing viewstate of the GridView
When in Edit page mode, DNN dynamically injects a new UpdatePanel between the pane and the controls. I believe this causes the ViewState to be broken on postback -- probably due to the ViewState manager unable to match back with the control IDs since it shifted by a level??
Furthermore, if you have a module that dynamically loads other controls that contains UpdatePanels, it would break badly with a yellow page error message:
Cannot unregister UpdatePanel with ID 'XXX' since it was not registered with the ScriptManager. This might occur if the UpdatePanel was removed from the control tree and later added again, which is not supported.
I saw many people have reported it on the forum but never knew it's due to this dynamic injection of the UpdatePanel between controls:
https://www.dnnsoftware.com/forums/threadid/539307/scope/posts/problems-by-using-of-updatepanel-in-dnn9
https://www.dnnsoftware.com/forums/threadid/542352/scope/posts/updatepanel-exception-when-in-edit-mode
https://www.dnnsoftware.com/forums/threadid/540864/scope/posts/unable-to-edit-pages-after-upgrading-to-911
https://github.com/DNN-Connect/IdentitySwitcher/issues/22
Steps to reproduce
In our simple example, we have a MultiView control that switches between two different Views depending on the state. After a button postback, the MultiView does not retain the last selected active view when submitting in Edit mode. We also saw in the case of a GridView, it would be empty after postback.
In another example, create a module control that dynamically loads a 2nd control inside the Page_Init (i.e. call the LoadControl(...) method). The 2nd control contains an UpdatePanel. You will get the error message "Cannot unregister UpdatePanel..." just by loading the page in Edit mode.
The same controls all works fine in View page mode.
Expected behavior
The MultiView control should retain the last active view on postback. The GridView should retain its values.
Dynamically loaded control with an UpdatePanel should not break the page.
Additional context
I believe there was an attempt to fix this part of the problem, but i don't think the fix got merged in.
https://github.com/dnnsoftware/Dnn.EditBar/issues/23
Affected version

[x] 09.06.01 latest supported release
[x] 09.01.01

Affected browser

[x] Chrome
[x] Firefox
[x] Safari
[x] Internet Explorer 11
[x] Microsoft Edge (Classic)
[x] Microsoft Edge Chromium

I believe this is related to the SupportsPartialRendering flag on a module control.  If you are dynamically doing things this should NOT be set to true.

Hi @mitchelsellers, I don't think it will make a difference.
The root cause is in the CreateChildControls() implementation of the Dnn.EditBar.UI.Controllers.ContentEditorManager.cs file.  It injects its own UpdatePanel in between the tree hierarchy at runtime, which I think is a risky code to begin with. Due to the unwanted side effects, it then tries to compensate by attaching an unload event to the existing UpdatePanels and use reflection to force the ScriptManager to re-register the child UpdatePanels. I believe due to the the timing of the page life cycle, if you have dynamically loaded controls, this event would not get attached and therefore will break with the Cannot unregister UpdatePanel with ID 'XXX' since it was not registered with the ScriptManager error. In addition, it appears it can break the ViewState as well under some circumstances.
Correct me if I'm wrong but I didn't find anywhere that would make a difference in the code if SupportsPartialRendering would bypass this injected UpdatePanel.
Thank you

EditBar is no longer used

The original author in JIRA DNN-10523 mentioned the CreateChildControls() in the EditBar. I simply referred to it.
I think somewhere else in the DNN code is doing the similar UpdatePanel injection breaking the modules. I don't know where exactly but if I do a find for "updatePanel = new UpdatePanel", I found 2 more places where this is potential happening?:
DotNetNuke.Framework.Ajax.cs - WrapUpdatePanelControl(Control objControl, bool blnIncludeProgress)
DotNetNuke.UI.Modules.ModuleHost.cs - LoadUpdatePanel()

Right but it is ONLY wrapped if you opt in. Not by default.

I just checked the module is already set to supportsPartialRendering=false in the manifest and correctly registered as disabled in the DNN Extension, yet the problem is happening in Edit mode.

In DNN 9.6.1 when in Edit mode, the Dnn.EditBar.UI.Controllers.ContentEditorManager.CreateChildControls() definitely gets called. I was able to step through the lines in the Visual Studio debugger and I see the UpdatePanel line gets injected along with the reflection call trying to re-register with ScriptManager despite the supportsPartialRendering being false. I notice if a module has loads controls dynamically, the GetUpdatePanelsInPane(pane) won't find it so it wasn't able to re-register correctly leading the error.

Weird, I have multiple modules that work totally fine. Do you have a simple module example you could share?  Also was this a clean installation or an upgrade?

Sure. I created 2 simple modules to show the 2 different errors.
Dnn.UP.zip - The control has a 2 level nested child control (View.ascx loads Child.ascx dynamically, which in turn references Child2.ascx statically). Both child controls have UpdatePanels.
Notice in Edit mode, you get the Cannot unregister UpdatePanel error message. In debugging, I found the recursive GetUpdatePanelsInPane(pane) method did not find all the child UpdatePanel controls even though I can clearly see they exists in the object viewer (after drilling down the Controls hierarchy many times). So the issue might not be due to the dynamic loading nature of the child controls, but rather the bug is in the recursive method incorrectly skipping over some deep nested controls.
Dnn.UP2.zip - The View.ascx loads the Child.ascx dynamically.
Child.ascx has a MultiView control that is initialized to "View 2" on Page_Load by default (note the IsPostBack check condition). After you submit the button, it should stay on "View 2" if ViewState is preserved. This works fine in View mode. Notice in Edit mode, the MultiView is reset to none after submit. Is the ViewState lost?
Let me know if you have any questions. Thank you

I forgot to attach the source code. The previous 2 zip files are the compiled modules. Here are their sources:
Dnn.UP.Source.zip
Dnn.UP2.Source.zip

If i make the correction to the recursive code, I'm able to fix the first error.

But I still haven't figured out what causes the ViewState lost though.

The above recursive code is Dnn code or your module code. If you found a solution, a pull request would be nice so we can quickly understand what the change is...

The highlighted part in blue is my code fixing the error in the DNN recursive code. I haven't figured out the other problem (there are 2 issues here).

We're still working on it, but we're still stuck on the 2nd issue not working. I would appreciate any help or feedback here.


Update deployment titles

Description
Remove a & an & instance from the title of each deployment

The only solution that doesn't have it in the title is Caprover and imo it looks much better.
or to simplify it even further i think it would look better if the title is the name of the deployment only.

will be fixed in #566


Minor adjustments to spacing for logo and footer links

Based on recent changes to logo and footer links, needed to adjust spacing slightly of both to look natural.

Logo vertically centered more precisely
Footer links at bottom right justified right to be aligned with margin-right of each page

Before

After

I originally just added the className ternary to Footer.js based on what window.location.pathname is to adjust the class depending on if the user is logged in or out but that means that when logging out the class isn't adjusted because the Footer actually isn't re-rendered when each page loads (including logging out).  So using the className solution it was necessary to make sure the Footer renders upon logout.
It is probably possible (and perhaps cleaner) to use state for prompting re-renderings also as some other components use props.store.data.user.isSignedIn as the test for whether the user is signed in or not.  (like ManageTeams.js).  If that's cleaner to avoid the repetitive <Footer /> in app.js, I can adjust in that direction though the current solution as is achieves the desired outcome I believe.

Did a quick experiment using props.store.data.user.isSignedIn instead of window.location.pathname to decide the class and it worked so I committed that change. I like this way a little more as it is more consistent with other pages.


Location::search is not consistent between the server and browser

For example, on the client, this is the result:
?hello=world&foo=bar
But on the server, it's different:
hello=world&foo=bar

Thanks! I had to choose between including the ? in both and excluding it from both. I chose to exclude it from both by removing it on the browser side. This means any server-side logic should still work as previously. Browser-side logic with use_query etc. should work, because that was already using the browser's built-in searchParams iterator, not the raw search string. Libraries like serde_qs omit the opening ? because it's not a part of the search proper, so I decided to just remove ? from the search memo in Location across the board.

Appreciate this! I do think omitting the ? was the right choice.


feat(bindings/cli): Implement --config

Description:
Implements --config. Plugin paths that are explicitly relative (start with ./ or ../) are converted to an absolute path using the current working directory as the base.
Related issue (if exists): https://github.com/aspect-build/rules_swc/pull/149

Hi, any thoughts on this? It's blocking implementing plugins in rules_swc.


Conference: ZK Summit 10

Website
https://www.zksummit.com
Dates
20th September 2023
Location
London
TODO

[ ] Accommodation
[ ] Attendees to self-organise travel

Avi

Arrives LHR 09:30 Mon 18th September
Depart LHR 06:45 Thu 21st September

Accommodation
Avi

Z Hotel Holborn, 75 Kingsway, London, WC2B 6SR
Check-In:  Mon 18th September
Check-Out:  Wed 20th September

Marijan

Z Hotel Holborn, 75 Kingsway, London, WC2B 6SR
Check-In:  Tue 19th September
Check-Out:  Thu 21st September

Nick

Z Hotel Holborn, 75 Kingsway, London, WC2B 6SR
Check-In:  Mon 18th September
Check-Out:  Thu 21st September

Mark
???


4.0 | Proposal: improve consistency for PHP open tag tokenization

Current Situation
Inspired by #588
As things are, the tokenization of PHP open tag tokens is taken from PHP itself, without changes, but the tokenization in PHP itself is inconsistent in regards to how whitespace following the actual tag is tokenized.

Open tag / followed by
<?php
<? (with short_open_tags=on)
<?=

New line
new line included in T_OPEN_TAG
T_OPEN_TAG + T_WHITESPACE
T_OPEN_TAG_WITH_ECHO + T_WHITESPACE

Trailing spaces + new line
one space included, followed by T_WHITESPACE
T_OPEN_TAG + T_WHITESPACE
T_OPEN_TAG_WITH_ECHO + T_WHITESPACE

No space
**
T_OPEN_TAG
T_OPEN_TAG_WITH_ECHO

One space
space included in T_OPEN_TAG
T_OPEN_TAG + T_WHITESPACE
T_OPEN_TAG_WITH_ECHO + T_WHITESPACE

Multiple spaces
one space included, followed by T_WHITESPACE
T_OPEN_TAG + T_WHITESPACE
T_OPEN_TAG_WITH_ECHO + T_WHITESPACE

** How the <?php tag without space after it is tokenized depends on whether short_open_tags=on.
With short_open_tags=off: tokenized as T_INLINE_HTML.
With short_open_tags=on: tokenized as T_OPEN_TAG <? + T_STRING for php + whatever came after.
:point_up: In my opinion, the above is correct and will not be changed as <?php without any space after it, should be considered a parse error anyway.
The Problem
The short of it, is that short open tags - <? - and short open echo tags - <?= - are tokenized consistently as just the plain tag without any whitespace inclusion.
But the token for long open tags - <?php - may include either a new line or a single space after the tag itself, but only that. Any additional whitespace after that is tokenized as a separate T_WHITESPACE token.
This inconsistency means that sniff writers need to remember this unintuitive peculiarity and have to take it into account whenever a sniff needs to deal with PHP open tags.
This also makes the code in these type of sniffs more complicated and increases the cognitive load.
I suspect there will be numerous sniffs out there (including in PHPCS itself) which don't take the difference in tokenization between the long and short open tags into account correctly. This will, in most cases, only be problematic for files using short open tags, as most sniffs default to expecting long open tags.
However, this also means that those sniffs will be buggy if they are used to scan files with short open tags (with short_open_tags=on).
Proposal
I'd like to propose to streamline the tokenization of PHP open tags to never include whitespace.
In practice, only the tokenization of long open tags <?php is affected by this change proposal, as those would no longer include a new line/single space.
I expect the impact of this change on existing code to be relatively low as there are not that many sniffs which specifically deal with PHP open tags.
I also expect that the majority of the affected sniffs will be PHPCS itself (as external standards rarely have the need for extra sniffs for open tags as PHPCS itself has most of the things which could be examined covered via the existing sniffs).
Planning
As this would be a breaking change for sniffs which examine the T_OPEN_TAG token, this change would go into PHPCS 4.0.
The change going into PHPCS 4.0 also means we don't have to consider the impact of this change on CSS and JS tokenization and sniffs anymore.
Opinions
Please let me know if you have any concerns about this proposal or any suggestions for alternative approaches.
/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg

Looks like the PHP close tag also may include whitespace. As there is only one flavour of PHP close tags, I'm not sure anything should change for close tags though.

I like the idea of consistency. It makes sense to me that the token for a PHP tag (both opening and closing) contain the tag only and no white-space.


Negated version of toMatchElement does not take into account element visibility

🐛 Bug Report
expect(instance).toMatchElement(selector, {visible: true}) does not take into account element visibility as you'd expect it to.
To Reproduce
Given a page with a hidden div containing the text 'Foo' (for example,
Foo
), a test with the following assertion should pass:
expect(page).not.toMatchElement('div', { text: 'Foo', visible: true })

However, the test fails with:
Element div (text: "Foo") found

Expected behavior
Based on the documentation for the toMatchElement method I'd expect setting visible: true to only consider elements that are visible – as such it should not match and the test should pass.
Link to repl or repo (highly encouraged)
Adding this test to packages/expect-puppeteer/src/matchers/notToMatchElement.test.js demonstrates the issue:
    it('should match taking into account visibility', async () => {
      const main = await page.$('main')
      await expect(main).not.toMatchElement('div', { text: 'displayed element', visible: true })
    })

It fails with:
  ● not.toMatchElement › ElementHandle › should match taking into account visibility

    TimeoutError: Element div (text: "displayed element") found

Environment info
## System:
 - OS: macOS 11.6.1
 - CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
 - Memory: 436.18 MB / 16.00 GB
 - Shell: 5.8 - /bin/zsh
## Binaries:
 - Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
 - Yarn: 1.17.3 - /usr/local/bin/yarn
 - npm: 7.24.2 - ~/.nvm/versions/node/v14.17.6/bin/npm
## npmPackages:
 - jest-puppeteer: ^5.0.4 => 5.0.4

(Having checked out the main branch of jest-puppeteer and added a test for the same functionality, I'm reasonably confident this issue is still present in 6.0.0 and above)

Please upgrade to v8 and reopen submit a new issue if you experience this problem.


Support delete files in batch

Oh, one thing forgot. I think we should also add relative method for the client. It should be in IndyContentClientModule.

Oh, one thing forgot. I think we should also add relative method for the client. It should be in IndyContentClientModule.

I will add it later, thanks.

I will merge this for the test. And will provide another PR if needed. thanks all.


[$250] Submit expense - Not here page displayed in the background after a refresh

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 9.0.16
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers<EMAIL_ADDRESS>Issue reported by: Applause - Internal Team
Action Performed:

Navigate to staging.new.expensify.com
Click on FAB
Click on start chat
Refresh the page
Observe everything works as normal
Click on FAB
Click on submit expense
Refresh the page

Expected Result:
Everything should work as normal just as it is on step 5
Actual Result:
Not here page gets displayed in the background when submit expense page is open on the right and user refreshes the page which is a different behavior that doesn't happen when other right hand pages are open
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[ ] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence

https://github.com/user-attachments/assets/de7e7e38-32c3-4cb2-af2d-c6d2fa6b54d0
View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a60ec1cb3ed7a96b
Upwork Job ID:<PHONE_NUMBER>663945809
Last Price Increase: 2024-08-06
Automatic offers:

nkdengineer | Contributor | 103466921

Issue OwnerCurrent Issue Owner: @sobitneupane

@joekaufmanexpensify  FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

We think that this bug might be related to #vip-vsp

Proposal
Please re-state the problem that we are trying to solve in this issue.
Not here page gets displayed in the background when submit expense page is open on the right and user refreshes the page which is a different behavior that doesn't happen when other right hand pages are open
What is the root cause of that problem?
When we submit an expense from FAB, the reportID is random but we call OpenReport API here and then a invalid report is stored in Onyx.
https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/pages/iou/request/step/withWritableReportOrNotFound.tsx#L69
Then the report screen with invalid reportID is added here when we open a RHP screen
https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L154-L156
https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L187
https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/libs/Navigation/linkTo/index.ts#L135-L141
What changes do you think we should make in order to solve the problem?

OPTIONAL: We should prevent calling the OpenReport API in the create flow

https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/pages/iou/request/step/withWritableReportOrNotFound.tsx#L69

We should safely check whether the reportID exists or not here since some not found reports can be stored in Onyx

if (ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.reportID) {
    return {name: SCREENS.REPORT, params: {reportID}};
}

https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L154-L156

We can add the money request screen SCREENS.MONEY_REQUEST.CREATE here

https://github.com/Expensify/App/blob/2eb65f8a666d0337ff6fd0d2ea0e7669b9d6aa6e/src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts#L24
What alternative solutions did you explore? (Optional)

@joekaufmanexpensify this is simlar to https://github.com/Expensify/App/issues/46713 and my proposal can fix it too, but RCA a little bit different

Got it. If we can fix all of the various cases in one issue, that feels ideal IMO

Pending proposal review

@sobitneupane mind taking a look here?

Thanks for the proposal everyone.
@nkdengineer Regarding the changes you have proposed,

OPTIONAL: We should prevent calling the OpenReport API in the create flow

It was added by this PR. Won't removing it cause regression?
The third change you proposed has already been implemented in the latest main branch.

Thanks for your proposal @tsa321
We cannot implement the change you suggested. In some scenarios, when opening the app through a deep link, it's necessary to open the report in the background. For instance, if we're currently viewing a report at /r/abc and attempt to open a deep link like /create/submit/start/1/cde/manual, the report cde needs to be opened in the background.

Thanks for the update
Proposal from @nkdengineer looks good to me.
🎀 👀 🎀 C+ reviewed

If your last action, for example, is submitting expense report A, then opening report B and clicking QAB, report A does not open; it stays on report B. Based on this, it is not necessary to open the report with the reportID(cde) in /create/submit/start/1/cde/manual in background. Therefore, the assertion that report A must open in the background is incorrect.

Just a note about this case: When we submit expenses to report A and then open report B, if we click on the QAB report B still appears. If we refresh the page, the report A is displayed as expected since we access the deep link has reportID as report A.

PR in review

PR merged and on staging

Hmm, automation did not work here. This went out to prod on the 19th, so is actually due today. Will tackle payment in the next day or so!

Payments here are:

$250 to @nkdengineer for PR via Upwork.
$250 to @sobitneupane for C+ via NewDot.

All set to issue payment!

@nkdengineer $250 sent and contract ended

Closing as only thing left here is for @sobitneupane to request payment via NewDot!

$250 approved for @sobitneupane


Setup translations management/admin panel for Bank Android

We need a solution that integrates with Github and provides an easy admin panel for potential contributors in the future.

Done.


Issue 271 (add function ENgetheadlossformula)

This looks fine, but adding a function to retrieve each individual parameter has me concerned. This will lead to a cluttered API that will be difficult to use. We should be thinking how we can get more functionality out of each API call we create. Grouping parameters together and returning larger chunks of data to the user. We have been experimenting with this in the swmm API and the output APIs and it has been working out pretty well.

For the section [OPTIONS] we have the functions ENgetoption, and ENgetflowunits if I not mistaken. So it's better to include this function in ENgetoption?

Yes, including this in ENgetoption() is preferred to creating yet another API call.

@eladsal, now? Thanks!

@Mariosmsk have you seen my earlier comment?

I have added some parameters from adjustdata. I'm not sure if it's okay.

@Mariosmsk when a user changes the head loss equation then all of the existing pipe roughness coefficients (Kc) become meaningless (i.e., a Hazen-Williams C-factor is much different than a Darcy-Weisbach roughness height). So modifying the reaction rate coefficients at the time that the head loss equation changes, based on the old Kc values, is problematic.
Whenever a change is made to the head loss equation a corresponding change needs to be made to all of the model's Kc values. One way (maybe not the best way) to do this is to define a set of default values for each equation (KC_HW, KC_DW, KC_CM) and set all Kc values to one of these when an equation change is made.

default values are hard... I will just point out that "friendly" model-user-facing features like, for instance, stupid-proofing the act of changing headloss formulation, may well be considered outside the responsibility of the engine API. Access to the data model and its important features (in my view) should be a separate concern from ensuring that the user knows what they are doing. Specifically, auto-changing roughness coefficients looks more like a application-level feature than an engine feature.

I agree with @samhatchett. I think is better to set headloss formula and the user change parameters coeff. as he wants.

So, when the user changes the headloss formula a warning could be displayed to inform the user that the current pipe roughness coefficients are meaningless if the formula is changed and further actions are required. e.g.:
#define WARN7 "WARNING: Pipe roughness coefficients may become invalid when headloss formula is changed. Further actions may be required."

Ok, so you change the headloss formula, the pipe roughness coefficients become invalid, and the internal state of the engine is essentially corrupt (it can't produce meaningful results). The user gets a warning and then what? How is the user going to correct this situation?
They would need to iterate over all the pipes in the network and recalculate the roughness coefficients for each of them. Would any other internal state of the model need to be updated?
Does the toolkit provide the necessary functionality to bring the model back into a valid state where it can produce meaningful results?

Added get option for headloss formula here #293. Maybe leave open this issue for the set of headloss formula.

Closed by #293


Bad hyperlink URLs in Moltres part of website

I'm submitting a ...

[x] bug report
[ ] feature request
[ ] question

Expected Behavior
In the Moltres part of the website, the URL of any hyperlink that intends to direct a user to a resource on the MOOSE website should correspond to said resource. Clicking on this link takes the user to the intended resource.
Actual Behavior
In the Moltres part of the website, the URL of many hyperlinks that intend to direct a user to resources on the MOOSE website do not correspond to said resources. Clicking on these links returns a 404 Not Found error.
Steps to Reproduce the Problem

Go to the Moltres part of the website and all of its related pages (wiki, input example, etc.)
Click on any hyperlink that appears to direct a user to a resource on the MOOSE website.

Specifications

Brower: Firefox v81.0 (64-bit)

How can this issue be closed?
This issue can be closed when all hyperlinks have URLs that correspond to the appropriate resource on the MOOSE website.

Thanks for noticing.
Cause: MOOSE has changed their website layout. What used to be in the wiki is now in main menu content. Here are some links to replace the current ones with:
Main page (not a wiki:) mooseframework.org
Tutorial: https://www.mooseframework.org/workshop/#/
Examples: https://www.mooseframework.org/examples/index.html


Shortest common supersequence

Added the code for Shortest common supersequence.
@Rohan27s  please review

mention the issue

Fixes: #167

@Rohan27s I will add the time complexity and also examples

@Rohan27s @tarun26091999 please review


Issue with generating Setters and Getters

Hi Team,
I am trying to exclude setters and getters by setting
includeGetters = false
includeSetters = false
but still, it is generating after setting false also. The default value is not overriding

What version are you using, and are you using grade?
I think this may be a bug. Could you try adding includeAccessors = false?

includeAccessors will be gone in 1.0.0 because of problems like this, so I'm closing this.


UI: Needs dark mode support

Need color set for dark mode. Currently, it's off-white text on a white background.

perhaps we start with a set of default dark styles that's the inverse of the default light styles: https://stackoverflow.com/a/66965199/185286

this is what I got with a combo of inversion and some hand tweaking:


Add warnings

What's this PR do?
This PR adds the concept of warnings to the project without causing a breaking change. The interface of formContainer now simply takes in a type option, like so
isRequired('username', 'please enter your username'), // error by default
isRequired('username', 'you might want to enter a username', ValidationType.Warning),
isRequired('password', 'please enter your password', ValidationType.Error)

Which generates validationErrors and validationWarnings in the validate wrapper.
Where should the reviewer start?
Probably

validate.ts
validators.ts

How should this be manually tested?
I've written a few extra tests, which should help. But on top of this I compiled the source and ran it in one of my projects and it seems to work well.
What are the relevant tickets / issues?
#33

Coverage increased (+2.7%) to 85.246% when pulling 98610b446ab7ad47ef76a3a8cb5d9bc1f5cd741a on issue/33-add-warnings-no-breaking-change into a966415e9fb9f90ff5645d4b68cacf87b7d4401b on master.

Codecov Report

Merging #39 into master will increase coverage by 2.83%.
The diff coverage is 86.95%.

@@            Coverage Diff             @@
##           master      #39      +/-   ##
==========================================
+ Coverage   82.71%   85.55%   +2.83%
==========================================
  Files           4        4
  Lines          81       90       +9
  Branches       13       15       +2
==========================================
+ Hits           67       77      +10
+ Misses          9        8       -1
  Partials        5        5

Impacted Files
Coverage Δ

src/validators.ts
100% <100%> (ø)
:arrow_up:

src/validate.ts
81.48% <82.35%> (+11.48%)
:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a966415...3f15be7. Read the comment docs.

nice one!
I've amended your branch a bit to fix some typings error that I had, have a look

I've tested again with the updates and it still works for me. Happy to merge?


Prisma2: BatchPayload type not generated

It seems that the BatchPayload type is not automatically generated for Prisma2/nexus projects.
In Prisma1, it was automatically generated as
type BatchPayload {
  count: Long!
}

Not sure if this is type/feature issue or a type/bug issue.
Nexus/nexus-prisma automatically generates the type BatchPayload in the GraphQL schema, but not in the typegen output, i.e.
  outputs: {
    typegen: join(__dirname, '../generated/nexus-typegen.ts'), // BatchPayload missing
    schema: join(__dirname, '/schema.graphql'),                // BatchPayload present
  },

So, you basically have to define BatchPayload manually as
export const BatchPayload = objectType({
  name: 'BatchPayload',
  definition(t) {
    t.int('count');
  },
});

and everything works alright.
Feel free to close this issue if that's the desired behaviour.

Fixed by #441


Add --scan-first option (same as --check-first in rclone)

Output of restic version
restic 0.15.1 compiled with go1.19.5 on linux/amd64
What should restic do differently? Which functionality do you think we should add?
As for now, restic backup has 2 choices: either don't scan source at all or scan concurrently with backup phase.
It would be useful to have option like --check-first in rclone, which is self-explanatory: scan first, than copy/sync (in case of rclone).
What are you trying to do? What problem would this solve?
It's great feature for cases where progress is useful, but metadata scanning is costly or starves IO (e.g. HDDs, which like sequential reads).
Did restic help you today? Did it make you happy in any way?
Restic is great.

What is the problem with using backup - -no-scan if you're worried about IO contention (which is very likely less of a problem than you might assume)? Yes, it won't provide a total number of files and the backup size, but that is the only difference.

It's not really much a problem, but in my understanding it should be easy to implement, and it helps when backuping large datasets, so you see sort of progress and eta, and hdds (at least, external ones) don't try to make different scary sounds from a lot of random reads under full load.
IMHO, it's not much a important feature, comparing to my second idea, it's just sort of "nice to have" one.


saved_snippets: Add keyboard UI for inserting a saved snippet.

Adds Ctrl+' keyboard shortcut to open saved snippet dropdown.
Fixes #31829.

Screenshots and screen captures:

Self-review checklist

[x] Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

[ ] Explains differences from previous plans (e.g., issue description).
[ ] Highlights technical choices and bugs encountered.
[ ] Calls out remaining decisions and concerns.
[ ] Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

[x] Each commit is a coherent idea.
[x] Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

[x] Visual appearance of the changes.
[x] Responsiveness and internationalization.
[ ] Strings and tooltips.
[x] End-to-end functionality of buttons, interactions and flows.
[ ] Corner cases, error conditions, and easily imagined bugs.

Do we want the popover to appear next to the cursor, or centered? It feels weird having it appear next to the send button.

Centered (and without the menu underneath) is what I suggested on CZO.


Error: Processed image is too large for the JPEG format

My Code
// sharp version 0.28.3
const sharp = require('sharp')
const fs = require('fs')
const path = require('path')
const imgTransformer = sharp().resize(2000).jpeg({
  quality: 100,
  chromaSubsampling: '4:4:4'
})
const write = fs.createWriteStream('./adsadas.jpg')
const read = fs.createReadStream(path.resolve(__dirname, './96313250-89bd-4161-bad2-678cac76e1db.png'))
read.pipe(imgTransformer).pipe(write)

throw error message:
Error: Processed image is too large for the JPEG format
source Image url:
链接: https://pan.baidu.com/s/1jBW_NSbvoG_ROsZJVNxCoQ?pwd=6vhv 提取码: 6vhv
i need your help,your can download this image and run code

The JPEG format supports up to 65535 pixels for each dimension. I can't access your original image, but from the thumbnail it appears to be a very tall image, so when resized to 2000 pixels wide, the required height will probably be much greater than 65535 pixels.

The JPEG format supports up to 65535 pixels for each dimension. I can't access your original image, but from the thumbnail it appears to be a very tall image, so when resized to 2000 pixels wide, the required height will probably be much greater than 65535 pixels.
Thank you.


WebApp:staticwebapp - [P2] enable and remove managed identity

Describe the solution you'd like
Enable managed identity. See az functionapp identity assign for optional params.
az staticwebapp identity assign -g $group -n $name

Remove managed identity. See az functionapp identity remove for optional params.
az staticwebapp identity remove -g $group -n $name

Additional context
Needed for Static Web Apps GA

Thanks for your feedback. We will check and consider for future milestones .
@panchagnula FYI.

Managing identity via CLI would require updating azure-mgmt-web sdk. Blocked on this for now

Closing since this feature was added to the latest release


how to integrate a typescript nodejs part

@albertlast
Javascript is very good since i can used on frontend, backend and his async nature,
but it's not typesafe like java, so the best mix of both would be typescript in my eyes,
could you add example here how to integrate a typescript nodejs part

I thought this would make a good standalone topic, so I will cover it in the ABAP_Freak->Show( ) next Tuesday, July 14th at 9AM US Eastern. https://www.youtube.com/watch?v=z6QDhCr7IA8

very inspiring you show
the importance information for me was cds2types: https://www.npmjs.com/package/cds-routing-handlers
also an example project already exists: https://github.com/HeneryHawk/cap-bookshop-typescript
and a blog exists about this: https://blogs.sap.com/2020/05/22/taking-cap-to-the-next-level-with-typescript/

I also added the blog and other Typescript links to the description section of the YouTube video as well. @albertlast  thanks for the suggestion of this topic. It was a very good idea. I know I had a lot of fun researching the topic.


Add package.yaml to support indexing on https://haindex.org

Hey there,
I'm currently building a user generated index for Home Assistant custom components and lovelace cards: https://haindex.org
To support the index with some metadata, I've added a package description file to this repository.
It would be great if you accept the pull request and head over to https://haindex.org to get the extension indexed. Just log in with your GitHub account and submit the repository URL to be indexed.
Thanks a lot!
Best, Jens

Awesome. Thanks for your support.