Added option to get user's current location

Details

This is a follow-up and builds on top of the reverted PR https://github.com/Expensify/App/pull/25990.
In https://github.com/Expensify/App/pull/25990, we added the feature to get the current location for a user. It was merged but the QA team found some issues while on staging.
These were the issues

https://github.com/Expensify/App/issues/26482
https://github.com/Expensify/App/issues/26478
https://github.com/Expensify/App/issues/26483
https://github.com/Expensify/App/issues/26485
https://github.com/Expensify/App/issues/26484

And these were the solutions

This issue was originally introduced in https://github.com/Expensify/App/pull/25974, but was not detectable because there was no elements under the AddressSearch component, with out changes it became detectable. Backstory -> We added a predefinedSpaces option to AddressSearch component in https://github.com/Expensify/App/pull/25974 to show the recently selected waypoints as suggestions in AddressSearch. When the text input in AddressSearch was empty, we were scaling (transform [{scale: 0}]) the address suggestion popover to 0 but the suggestion list was still taking space, causing the big space b/w AddressSearch and Use Current Location button. In this PR, we are showing the suggestion list even if the text input is empty, this is because we are giving predefined spaces to AddressSearch. (I suggested this here as option 1)
We are now blocking the 'Use current location' button when the user is offline (discussion here)
This is an issue with Chrome, explained in this comment. The original ticket was closed.
https://github.com/Expensify/App/pull/26500 seems to have fixed this (more context) - It was not related to our changes but I have still shown its working in test videos.
We removed the desktop-specific direct API call to geolocation and are now using navigator with the hardcoded token, discussion here

Fixed Issues

$ https://github.com/Expensify/App/issues/25855
PROPOSAL: https://github.com/Expensify/App/issues/25855#issuecomment-1693745805
Tests

Click on Fab -> Request Money -> Press Distance Tab
Click on any waypoint
On the waypoint editor page, click on the "Use current location" button
a. If location permission is disabled, verify that you see the error -> "It looks like you have denied permission to your location. Please allow location permission in settings and then try again.". Clicking on the "allow location permission in settings" should open https://help.expensify.com/ on web/desktop and app settings on ios/android native.
b. If location permission is enabled but there is some issue getting location, verify that you see the error => "We were unable to find your location, please try again or enter an address manually". To emulate this we can make our network speed extremely slow so we get a timeout issue and in turn the error or set the timeout to a lower value manually in code and test.
c. If you see any of the errors mentioned in 3a and 3b, navigate back, come again to this same page, and verify that the error message gets removed when navigated to the page.
d. If location permission is enabled, verify that the address field changes the text to "Your Location", and clears any errors on that page, and navigates you back to the distance page. Verify the waypoint edited with the text 'Your Location' and Mapbox zooming into your location.
Again go to a waypoint editor, open the address search, and confirm that the 'Your location' text does not appear in suggestions.
Go to any waypoint editor, and select an address from the suggestion (you will be navigated to the distance request screen). Now go to the same waypoint again, select the address text, and remove it. Verify that there are no large spaces b/w the address search input and the 'Use current location' button.
Go to any waypoint editor, type some address, and select it from suggestions (you will be navigated back to the distance request screen). Verify that you see the map zooming into the selected location
Go to any waypoint editor and turn off the network. Verify that the 'Use current location' button is grayed out and is not clickable.

Important:

On Chrome after disabling and then re-enabling location permission you would have to either close and reopen the tab or reset website permissions.

We have to reload the page, whenever we change location permissions in Web/mWeb Chrome/Safari.

If testing mWeb Safari in an IOS stimulator, when you disable the location permissions then it's very hard to get safari in the stimulator to allow location permissions (changing the settings does nothing most of the time). A reliable way is to reset the stimulator (I have shown this in the mWeb Safari test video). However, on a real iOS device with Safari, changing the settings works and we don't have to do extreme steps for it to work.

[ ] Verify that no errors appear in the JS console

Offline tests

Only steps 3d, 5, and 6 don't apply in offline. All other steps are the same as in the "Tests" section above.
QA Steps

Same as the "Tests" section above.

[ ] Verify that no errors appear in the JS console

PR Author Checklist

[x] I linked the correct issue in the ### Fixed Issues section above
[x] I wrote clear testing steps that cover the changes made in this PR

[x] I added steps for local testing in the Tests section
[x] I added steps for the expected offline behavior in the Offline steps section
[x] I added steps for Staging and/or Production testing in the QA steps section
[x] I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
[x] I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
[x] I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).

[x] I included screenshots or videos for tests on all platforms
[x] I ran the tests on all platforms & verified they passed on:

[x] Android / native
[x] Android / Chrome
[x] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[x] MacOS / Desktop

[x] I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
[x] I followed proper code patterns (see Reviewing the code)

[x] I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
[x] I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
[x] I verified that comments were added to code that is not self explanatory
[x] I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
[x] I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

[x] If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message: https://expensify.slack.com/archives/C01GTK53T8Q/p1693593279760999?thread_ts=1693582383.407909&cid=C01GTK53T8Q

[x] I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
[x] I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
[x] I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
[x] I verified the JSDocs style guidelines (in STYLE.md) were followed

[x] If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
[x] I followed the guidelines as stated in the Review Guidelines
[x] I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
[x] I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
[x] I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
[x] I verified that if a function's arguments changed that all usages have also been updated correctly
[x] If a new component is created I verified that:

[x] A similar component doesn't exist in the codebase
[x] All props are defined accurately and each prop has a /** comment above it */
[x] The file is named correctly
[x] The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
[x] The only data being stored in the state is data necessary for rendering and nothing else
[x] If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
[x] For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
[x] Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
[x] All JSX used for rendering exists in the render method
[x] The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions

[x] If any new file was added I verified that:

[x] The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory

[x] If a new CSS style is added I verified that:

[x] A similar style doesn't already exist
[x] The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))

[x] If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
[x] If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
[x] If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
[x] If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
[x] If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
[x] If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
[x] I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Chrome:
https://github.com/Expensify/App/assets/68777211/648eaf3c-51ad-4252-afcb-95274f248a0b
Safari:
https://github.com/Expensify/App/assets/68777211/4ca23486-4bfb-47fc-b3b0-c782863d1f28

Mobile Web - Chrome
https://github.com/Expensify/App/assets/68777211/24b5114c-ab33-43ad-a3bf-ab062ec53e24

Mobile Web - Safari
https://github.com/Expensify/App/assets/68777211/ea7751be-1e5d-4842-92d4-1f0ec5e68b7c

Desktop
https://github.com/Expensify/App/assets/68777211/e3be3366-89d3-4f17-bcc2-284dc2eb5330

iOS
https://github.com/Expensify/App/assets/68777211/1bc2d219-6d0a-4625-a66b-8b15697e6faf

Android
https://github.com/Expensify/App/assets/68777211/f74c4771-5516-468b-930d-391fcfe51212

@huzaifa-99

In https://github.com/Expensify/App/pull/25974, we added the feature to display recently selected waypoint in the AddressSearch component, but in the PR, we are supporting the current location, so when ever current location is used, the address search will show 'Your Location' as a suggested option, Should we filter it form the suggestion list?

Is the location associated with "Your Location" suggestion is the location when the user last used the current location option?
Yes I think we need to either filter out that option or make sure that current location is not saved as a recently-used address.

Should we display a "You appear to be offline" message on this page?
I'm asking the internal product team about this. I'll get back to you once they decide on this 🙇

How do we want the error text size/color to be displayed?
I believe the left one is the current one? Where did you come up with the right choice / text?

Should we display a "You appear to be offline" message on this page?
I'm asking the internal product team about this. I'll get back to you once they decide on this 🙇

The internal team got back to me. Yes we want to display "You appear to be offline" message

@huzaifa-99

I think we need to either filter out that option or make sure that current location is not saved as a recently-used address.
We can do any of that, I think not saving it as a recently used address might be better if that sounds good?.

I also agree with you. Let's not save the "Current location" as a recently-used address 👍

@huzaifa-99

How do we want the error text size/color to be displayed?

I'm so sorry I'm handling several issues right now, and I'm hazy on the context behind this question 😓 Where did you get all these texts?

@huzaifa-99 haha now I see your question. The right screen shot is a style example (I thought you're asking about changing the error text).
Let's use the prevent pattern (the right one i.e. smaller + gray)
@huzaifa-99 and I'm sorry for the late response. I'm handling several issues right now 😓

Also, I used the Spanish translations that you provided from here. I just want to confirm if the link-area/blue-text of the Spanish translation is semantically correct and the overall Spanish translations. Can you please tag someone from the internal team who can confirm these?

Confirming right now

Ok so I tested and it works perfectly on IOS native with a physical device. Sidenote: Contributors that are not part of the Expensify dev team for IOS cannot directly install the local dev app on IOS, I am using a free dev account so I had do to some modifications to the dev team config, app bundle ID, and some permission unrelated to this PR to install the app on IOS.

@huzaifa-99 I apologize for the slow response. So you cannot build iOS code in XCode and test on a physical devise?

Very important: we have to enable system-wide location/GPS permission for geolocation to work on native - I show this in the Android video.

Thank you for bring my attention to this detail. Although the message is not ideal (We were unable to find your location, please try again or enter an address manually), I think we don't have a way to differentiate the reasons for the location being unavailable (because of error code 2)?
In that case, the current behavior is perfectly fine 👍

I actually tested on an IOS physical device, by changing some configuration for "dev team" in Xcode @hayata-suenaga

Ah I didn't know you have to change the configuration to be able to build the iOS on a physical device. I gonna bring this to the internal team so that the experience of contributors will become more smooth
I really appreciate you taking extra steps to really make sure everything works correctly. I'm constantly impressed by your hard work 💯

@huzaifa-99 I just read this part

TLDR: Do we want to display a prompt to enable system-wide location on Android? IOS already handles that.

I think that's an amazing addition. Let's include that 🙇

Actually, there is another error message for code = 2 "No location provider available". This (in our case) means gps/location is not turned on

For the error message in case the user denies the system wide location, we can keep the current message 👍

@huzaifa-99 feel free to DM me on Slack (this is usually not allowed, but this is an exception) or ping me on #expensify-open-source if you need my input 👍

I have updated the Android implementation to now show the GPS/location enable prompt. Just for a heads-up, I suggest we test iOS/Android on physical devices.
Also updated the test videos for iOS/Android.
cc: @hayata-suenaga @parasharrajat @narefyev91

@huzaifa-99 please check why lint and ts jobs are failing. I will start new round of testing

@huzaifa-99 why does it take such delay - between clicking My location and render the map? is it intentional?
https://github.com/Expensify/App/assets/28352309/85fc26ea-1999-46b3-a279-b824a84c39a9

@parasharrajat maybe makes sense to fix next button in the bottom as we have such behaviour in most places? Now when we add new points next button is moving with map

@huzaifa-99 @parasharrajat seems this PR https://github.com/Expensify/App/pull/26525 introduced new regression - we can't move map to the right
https://github.com/Expensify/App/assets/28352309/3b849f96-e232-4f51-a3f5-46c001c2b73f

if start / end - has the same address inside stop - green line will not be shown
https://github.com/Expensify/App/assets/28352309/1d342caa-c42b-4630-b197-0f4dead9ba77

Green line is not shown - when coming from offline to online (steps - change a start or end during offline, go online)
https://github.com/Expensify/App/assets/28352309/91b5ff3e-dee7-4dba-929e-8b89104b4906

Get animation warning trying to swap to left on ios
https://github.com/Expensify/App/assets/28352309/88221106-6eb2-4240-a582-ff4485421b64

Yup I'm fully ok if all i found will not influence to move forward here :-)

Seems when i click back - during fetching location - section will be automatically closed - when location will be fetched
https://github.com/Expensify/App/assets/28352309/1df92607-5e5e-4952-91f4-36c91243dfdc

I think it's unrelated to our changes, I also noticed that the map actually has some more issues related to animation/load. I remember one where if a user selects an address while the map hasn't loaded it will give a promise rejection error.

@huzaifa-99 is this still reproducible? This results just in a console error, right?

Get animation warning trying to swap to left on ios
Screen.Recording.2023-09-04.at.15.20.39.mov

@narefyev91 could you report this bug in the expensify-bugs please?

@huzaifa-99 why does it take such delay - between clicking My location and render the map? is it intentional?
Screen.Recording.2023-09-04.at.13.05.36.mov

This issue is also present when you click a suggested address while the device is under a bad internet connection
https://github.com/Expensify/App/assets/98560306/af8f3b53-6ad9-4715-8b5f-3d1ceb8cb859

I believe @hayata-suenaga will be online in a few hours to confirm https://github.com/Expensify/App/pull/26546#issuecomment-1705181933, then will add the change. Or should I add it and we can revert it later if needed?

@huzaifa-99 I created a separate GH issue for this. We don't have to handle this in this PR because this PR is already big.

if start / end - has the same address inside stop - green line will not be shown

Hm, looks like a separate bug (unrelated to our PR). Maybe there is an issue for it already?

Yep
@narefyev91 I really appreciate your thorough testing, but please focus the testing on the part changed in this PR. If you find other bugs, please report them in the #expensify-bugs 🙇

Seems when i click back - during fetching location - section will be automatically closed - when location will be fetched

This is a really good find good job @narefyev91

The zoom scale is too high on the map when both start and stop points are your location.

In some locations yes (for my current location it looked fine), looks like it also depends on the waypoints b/w start and finish locations. I believe its an issue with the map tho, and can be dealt with as a separate issue.

Yep this is known and we can handle in a separate issue if we decide to do so 👍

We already did @hayata-suenaga

Does the loading handle both current location fetching and address fetching?
or just the current location fetching?

if start / end - has the same address inside stop - green line will not be shown

Hm, looks like a separate bug (unrelated to our PR). Maybe there is an issue for it already?

Yep
@narefyev91 I really appreciate your thorough testing, but please focus the testing on the part changed in this PR. If you find other bugs, please report them in the #expensify-bugs 🙇

Yup - in case this is a massive one PR - sometimes hard to see if that issues happened with current implementation or coming from something else. Just posted all my findings during testing - nothing else :-)

I love the steady progress we're making here ❤
@huzaifa-99 could you also check this comment when you have time 😄 ?

Ah, I already replied to that one https://github.com/Expensify/App/pull/26546#issuecomment-1708190235 @hayata-suenaga

I missed that 😓 yes I agree let's handle it in another issue

Sorry for coming into this late, but I think I like the label update the most:

However, I would suggest we change out the green icon to use our default white loading spinner while this loads.

On this screen here:

The line height of the error message seems way too big - there shouldn't be as big of gaps between lines. Can we use our standard line height here instead please?

@shawnborton
I think the issue with this approach is that the user can still click the "Save" button at the bottom of the waypoint editor page.
There is no "current location" button in the screenshot below, but you can see the "Save" button at the bottom
When the user clicks this "Save" button while the current location is being fetched, there will be an error. So we want to prevent the user from doing nothing while the current location is pending.
@huzaifa-99 @parasharrajat please confirm ⬆

@Expensify/design Please let me know which option is better here:

Disable the entire page with full page loader
Disable all UI elements on the page individually

I feel like the full page loader is too heavy of an approach.
In terms of disabling the Save button, we could do this, but I might recommend doing this without any explicit visual feedback. I feel like it's an edge case that someone would click "Current location" and then immediately try to hit Save before the location is fetched. So maybe we can block saving while things are being fetched, but I don't think we need to flash in any visual feedback that would make this experience more distracting than it needs to be for the 99% of users who wouldn't try to Save early.

@shawnborton I understand your reasoning for not blocking the whole page with a loader.
But I have to say that I'm still inclined with the full-page loader approach.
I have three reasons for this

There are two possible ways in which a user can enter an address/location. The user can type and select an address in the address field, or they can click "Use current location" button. We have to give an UI feedback for both scenarios while the address/location is being fetched. While you can give a UI feedback through current location button when the current location is being fetched, we don't have a good way to give a UI feedback when the address is being fetched. And it's probably better to use the same full-page loading feedback for both address/current location fetching.
I believe that it's better to make the whole page covered with a loader because we have to make several UI elements disabled anyway (@huzaifa-99 suggested that we also have to make the address search bar disabled)
I'm not in favor of disabling a button without any UI feedback as you suggested. The user might think the App froze if a button cannot be clicked.

Please let me know what you thing 🙇

While you can give a UI feedback through current location button when the current location is being fetched, we don't have a good way to give a UI feedback when the address is being fetched.

This is why I suggested that we use the same loading spinner on the suggested list popover. Basically replace all of the list menu options with the loading spinner while this is happening:

I feel quite strongly that we should try to avoid full page loading at all costs. It's a very heavy interaction to present to the user and it makes the perceived performance of the app feel like the app is sluggish and slow. Using in-line loaders feels way more smooth and presents less visual interruption to the user as they go through this flow.

I'm not in favor of disabling a button without any UI feedback as you suggested. The user might think the App froze if a button cannot be clicked.

I actually think we should let the user click the button but return an error when they do so. This is in line with our FORMS.md guidance:

Submit buttons shall not be disabled or blocked from being pressed in most cases. We will allow the user to submit a form and point them in the right direction if anything needs their attention.

cc @Expensify/design for more thoughts, too.

This is why I suggested that we use the same loading spinner on the suggested list popover. Basically replace all of the list menu options with the loading spinner while this is happening:

ah I think I didn't communicate the issue well 🙇

we don't have a good way to give a UI feedback when the address is being fetched.

With this, I meant after the user clicks the suggested address and until the user is taken back to the previous page (Distance Page) vs. when the address suggestion is being loaded. Please refer to the following video:
https://github.com/Expensify/App/assets/98560306/63aa39bc-02ab-46bf-8b10-1ecdb026dfb4

I feel quite strongly that we should try to avoid full page loading at all costs. It's a very heavy interaction to present to the user and it makes the perceived performance of the app feel like the app is sluggish and slow. Using in-line loaders feels way more smooth and presents less visual interruption to the user as they go through this flow.

I agree with this 100%. Full page loading also comes across as so much worse in our app, because it's already so so slow as it is.

I think for the loading of the current location, we can change the label.
We still need to decide what to do for the loading of a suggested address.

For the suggested address, how about my idea from here?

@shawnborton
I'm so sorry but there seems to be a confusing here 🙇 But this is what I'm taking about ⬇

With this, I meant after the user clicks the suggested address and until the user is taken back to the previous page (Distance Page) vs. when the address suggestion is being loaded.

You suggested that we don't wait for the address to be fetched before going back to the previous screen (Distance Request page), but there is a possibility that an error might happen while fetching the address.
We want to encourage the user to re-input the address in case an error occurs in the Waypoint Editor page.
What do you think the best way to let the user know that the address is being fetched 🤔

I am getting pretty confused. What exactly do you mean by the address getting fetched?
When the user goes to look for an address to enter, the suggested addresses box fetches addresses. This takes a short while.
Then once a user clicks on one of those suggested addresses, are you saying that there is even more loading that needs to happen?

Then once a user clicks on one of those suggested addresses, are you saying that there is even more loading that needs to happen?

yep 😄
https://github.com/Expensify/App/assets/98560306/4f4fe626-8197-47b7-80f6-4d43edbfea34

Hmm, I think we should make that click happen immediately - as in, we immediately take the user back to the previous page. Then from here, we add some kind of loading state to the map as it thinks while it loads the new address. Otherwise that delay feels almost like the app is broken or something.

@neil-marcellini I forgot who worked on the waypoint editor page, but do you know what we're doing here form the time the address suggestion is clicked to go back to the Distance Request page?
Are we fetching the coordinate of the address?

@huzaifa-99 @parasharrajat
I think we can ignore the loading of suggested address now and merge this PR.
Is it my review that is blocking the merge? 😓

@huzaifa-99 just checking in 😄 how are you doing fixing the crash issue?

@huzaifa-99 thank you so much for the update. I know how frustrating the process can be when trouble shooting these kind of bugs
I really appreciate your continuous efforts

@huzaifa-99 I think the last link is not working. could you provide the right link? 🙂

@huzaifa-99 have you made changes regarding the error code storage change?

Although the checklist has been completed before, because several changes were made since then, could you do another checklist @parasharrajat ?

Testing on chrome
https://github.com/Expensify/App/assets/98560306/2a927356-6350-4d9b-8967-4913ad154ad5
@huzaifa-99 I thought the design decision was not to use the whole page blocking loader

@huzaifa-99 while testing on desktop, I realized that I cannot click the "+" icon in the text composer.
The issue is also present on chrome. Can you retest and confirm? Maybe pulling the latest main will fix it?
https://github.com/Expensify/App/assets/98560306/a18fea1e-c35b-4e7f-a533-5af73133fe63

I'm having trouble testing on iOS, but I was at least able to confirm that I can click the "+" icon of the text composer on iOS
https://github.com/Expensify/App/assets/98560306/207b9f16-25b8-4947-8dd1-12175646f84d

I will approve and merge this as-is. If you choose to correct the few things I pointed out, please submit a follow-up PR to fix them.

:hand: This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@huzaifa-99 I think the "Use your location" button should display in the above suggestion list as designed here: https://github.com/Expensify/App/issues/27165#issuecomment-1717460142
cc @trjExpensify

🚀 Deployed to staging by https://github.com/tgolen in version: 1.3.79-0 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
success ✅

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
failure ❌

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
cancelled 🔪

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅

🚀 Deployed to production by https://github.com/jasperhuangg in version: 1.3.79-5 🚀

platform
result

🤖 android 🤖
success ✅

🖥 desktop 🖥
success ✅

🍎 iOS 🍎
failure ❌

🕸 web 🕸
success ✅


[SPARK-32706][SQL] Improve cast string to decimal type

What changes were proposed in this pull request?
This pr makes cast string type to decimal decimal type fast fail if precision larger that 38.
Why are the changes needed?
It is very slow if precision very large.
Benchmark and benchmark result:
import org.apache.spark.benchmark.Benchmark
val bd1 = new java.math.BigDecimal("6.0790316E+25569151")
val bd2 = new java.math.BigDecimal("6.0790316E+25");

val benchmark = new Benchmark("Benchmark string to decimal", 1, minNumIters = 2)
benchmark.addCase(bd1.toString) { _ =>
  println(Decimal(bd1).precision)
}
benchmark.addCase(bd2.toString) { _ =>
  println(Decimal(bd2).precision)
}
benchmark.run()

Java HotSpot(TM) 64-Bit Server VM 1.8.0_251-b08 on Mac OS X 10.15.6
Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
Benchmark string to decimal:              Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
6.0790316E+25569151                                9340           9381          57          0.0<PHONE_NUMBER>.0       1.0X
6.0790316E+25                                         0              0           0          0.5        2150.0 4344230.1X

Stacktrace:

Does this PR introduce any user-facing change?
No.
How was this patch tested?
Unit test and benchmark test:

Dataset
Before this pr (Seconds)
After this pr (Seconds)

https://issues.apache.org/jira/secure/attachment/13011406/part-00000.parquet
2640
2

Test build #128575 has started for PR 29731 at commit a74349b.

cc @cloud-fan @Ngone51 @maropu

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33200/
Test PASSed.

Test build #128575 has finished for PR 29731 at commit a74349b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128575/
Test PASSed.

Test build #128614 has started for PR 29731 at commit d568e11.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33238/
Test PASSed.

Test build #128614 has finished for PR 29731 at commit d568e11.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128614/
Test FAILed.

Test build #128616 has started for PR 29731 at commit 3f2e2a5.

Build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33240/
Test PASSed.

Test build #128617 has started for PR 29731 at commit 7f54640.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33241/
Test PASSed.

Test build #128617 has finished for PR 29731 at commit 7f54640.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Test build #128616 has finished for PR 29731 at commit 3f2e2a5.

This patch fails due to an unknown error code, -9.
This patch does not merge cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128617/
Test FAILed.

Build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128616/
Test FAILed.

Test build #128716 has started for PR 29731 at commit a98c38d.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33340/
Test PASSed.

Test build #128718 has started for PR 29731 at commit 94555ef.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33342/
Test PASSed.

Test build #128716 has finished for PR 29731 at commit a98c38d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128716/
Test PASSed.

Test build #128718 has finished for PR 29731 at commit 94555ef.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128718/
Test PASSed.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33368/
Test PASSed.

Test build #128744 has started for PR 29731 at commit abf2f7e.

Test build #128744 has finished for PR 29731 at commit abf2f7e.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test FAILed.

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128744/
Test FAILed.

retest this please

Test build #128750 has started for PR 29731 at commit abf2f7e.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/33373/
Test PASSed.

Test build #128750 has finished for PR 29731 at commit abf2f7e.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Merged build finished. Test PASSed.

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/128750/
Test PASSed.

thanks, merging to master!


Scope not provided to Authentication API

Describe The Bug:
After reviewing the documentation a second time, part of the Husqvarna documentation indicates the scopes of requests are required however nowhere in their documentation do they provide scopes when logging in.
To Reproduce:

Given a plugin in debug mode
When the plugin logs into the Husqvarna platform
Then the scopes are not provided in the outgoing request

Expected behavior:

Given a plugin in debug mode
When the plugin logs into the Husqvarna platform
Then the scopes are provided in the outgoing request

Related to #171


chore: update react imports

Hey, I just made a Pull Request!
Modifies React imports to remove import React from 'react', a required change for the upcoming React 19 migration.
https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
https://react.dev/blog/2024/04/25/react-19-upgrade-guide#installing
:heavy_check_mark: Checklist

[ ] A changeset describing the change and affected packages. (more info)
[x] Added or updated documentation
[ ] Tests for new functionality and regression tests for bug fixes
[ ] Screenshots attached (for UI changes)
[x] All your commits have a Signed-off-by line in the message. (more info)

Well this files tab kills the browser :D
Were you using the codemod for this? Seems to need api report updates and some more, too.


Update to v2.6.3

Checklist

[ ] Used a personal fork of the feedstock to propose changes
[ ] Reset the build number to 0 (if the version changed)
[ ] Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

@conda-forge-admin, please rerender

Oh, boy.  Adding Google's RE2 and python wrapper may be a nightmare.

Needs https://github.com/conda-forge/staged-recipes/pull/23439


Import of existing model fails due to missing control

Bug that popped up while I was updating an existing threat model. Will look at more details from logs/sentry later.

May have been fixed by d9e88711c3d24f3fa638b95b7a26d33e1400d01c - will need to test and see.


Errors out

[x] Plugin shows that I'm not authenticated. I am authenticated of course, my account is private and my API key is valid.

And for information: https://github.com/Lacro59/playnite-checkdlc-plugin/wiki/FAQ


autoCursor broken starting in v4.0.0

Refs https://github.com/mozilla-services/react-jsonschema-form/issues/851
We are using an UnControlled component with autoCursor={false} because this seems the most similar to the v2.X behavior. Without autoCursor={false}, every edit causes the cursor to jump to the end of the document (probably because we're resetting the value on every render). With autoCursor={false} in v3.0.7, the cursor behaves normally (for instance, advancing as the user types). However, in v4.0.0, even with autoCursor={false}, the cursor jumps to the end of the document at the end of every edit, as previously. As far as I can tell, autoCursor is essentially useless now (although I am not an expert in CodeMirror or even React so maybe I'm missing something).
I believe the change happened in https://github.com/scniro/react-codemirror2/commit/934521af9e89b58572d90e48ecdcf96a0760e95e#diff-168726dbe96b3ce427e7fedce31bb0bcL521.
Do you have advice on how best for us to upgrade to v4?

@glasserc Hey thanks for opening this up. I've read through this as well as the referenced issue and am curious as to why you are resetting value on an UnControlled component. The uncontrolled component does't care about the value whatsoever other than a starting one, as it's all managed internally and simply spit back out to the user on a cb (if needed). It's best to just give value a static string starting value and never worry about it again.
Perhaps you are wanting the Controlled variant if you are indeed wanting to manage value (perhaps this is coming down via redux, or similar). A few solutions come to mind...

Try not resetting the value prop.
If ^ is not possible, try using a Controlled component and manage the prop however you're managing state in your app
On my end, I can be more strict within componentWillReceiveProps on the UnControlled component.

I'm thinking of doing the last suggestion anyways as it makes sense, but I'd try the first two as well as I suspect you may be using UnControlled just a bit incorrectly. Either way, keep me posted with your findings and If I can reproduce this locally I'll get a fix published this weekend.

Hi, thanks for your advice. Some background: in our application, the user edits JSON in the editor, but periodically (when the JSON value being edited "changes", i.e. the parsed JSON object is different from the last parsed JSON object) we re-set the value to a pretty-printed version of the current JSON object. When this happens, ideally the cursor would stay "where it is", i.e. if the user has just typed the closing quote after a new property, then the cursor would still be located at this closing quote, even if the closing quote moved to a new location due to pretty-printing. However, this behavior seems hard, and all I really want is to have behavior that is similar enough to our old behavior, where the cursor goes to someplace nearby.
I think you're right that we are actually most like a Controlled variant, but I had some problems with the cursor moving to the end of the value whenever we re-format the JSON value. So I guess my bug should actually have the title "How can I maintain cursor position when I update the value?" As you can see from the linked issue, I tried using autoCursor={false} with the Controlled component, but that caused even stranger behavior -- the cursor remains fixed, even when typing new characters (which appear after the cursor, causing typed text to come out backwards). We weren't tracking the cursor position as a prop previously, but maybe that's the approach I should be following? I tried something like this but didn't see any difference in behavior from just not tracking the cursor state at all.

Hm okay, it seems like option 3 may be the best quick approach here. You can confirm this was not an issue in 2.x? I think I just need to be more strict in the handling of props on the uncontrolled component

@glasserc okay I may have found the issue by being more hands off with the uncontrolled component and letting cursor delegation largely be driven on it's own unless explicitly specified via autoCursor. The behavior should replicate the likes of v2.x
Here's the deal, testing this lib has been tough, and I can't reliably run my suite and assume this safely fixes the issue with the upmost certainty. I'm often left to manually regress this, which is okay for now. Also, even with reading over your use case a few times I couldn't exactly reproduce this on my end so I worked off a more basic scenario.
I pushed up scniro/react-codemirror2-test-pkg so this can be tested in isolation. Can you please install this and let me know if any of the issues on your end are resolved? Thanks

Yes, the behavior from 2.x is the same as the behavior of 3.x's UnControlled with autoCursor=false, and it's the same behavior I see from your test package (i.e. it fixes my issue, thanks!). I have a lot of sympathy for how difficult it is to maintain a project where people use it in unexpected ways -- if there's a more expected way for us to do what we're doing, I'm totally willing to do that too. I just couldn't figure out what that way was. If you have other branches or tests you'd like me to try, I'm happy to do that as well.

@glasserc Excellent news! I'm happy to hear this. Ideally I'll have feature branching in the project to just pull the source files but I've found folks who I can kindly convince to help me prefer quickly installing the tgz file 😸. Either way I definitely appreciate your help on this. I'm going to regress this a bit more this evening and publish up a new version for you to consume. Will keep you posted here when it's available.

@glasserc You should be all set with the 4.1.0 release. Please let me know if this fully resolves your issue as you tested with the earlier patch. Thanks for the collaboration on this!

Yes, it works great, thanks again!


Fix MCP2515 observe and document it

This PR fixes the MCP2515 observe method. Previously configuration mode was set (0x80), however according to the datasheet of MCP2515, the listen-only mode is 0x60.
The method observe is added to API.md for documentation.
Fixes #40.

Curious why this issue was closed? The library code still has the incorrect flag for the listen-only mode.


[leptonica] Update to 1.82.0 and fix link 'openjp2.lib' failed

Fixes #22938
Update leptonica to 1.82.0, fix old patches.

Use find_package(OpenJPEG CONFIG REQUIRED) instead of find_package(OpenJPEG REQUIRED)

This is not the problem. As there is no Find module for OpenJPEG, it will always find the config.

Don't add a new patch, but consolidate the existing patches.They add and modifiy the find_package calls
Make sure that linking uses targets where possible.
Add the corresponding find_dependency calls to the exported config.

@dg0yt Could you please review this PR again? Thanks a lot.

It is still not acceptable.

It mixes files and targets in INTERFACE_LINK_LIBRARIES. (And again, only targets need find_dependency.)
It even libopenjp2 in INTERFACE_LINK_LIBRARIES.

From x64-linux LeptonicaTargets.cmake:
set_target_properties(leptonica PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libgif.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libgif.a>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libjpeg.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libjpeg.a>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libtiff.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libtiffd.a>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/liblzma.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/liblzmad.a>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libjpeg.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libjpeg.a>;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libz.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libz.a>;m;WebP::webp;WebP::webpdemux;WebP::libwebpmux;WebP::webpdecoder;\$<\$<NOT:\$<CONFIG:DEBUG>>:${VCPKG_IMPORT_PREFIX}/lib/libz.a>;\$<\$<CONFIG:DEBUG>:${VCPKG_IMPORT_PREFIX}/debug/lib/libz.a>;m"
)

I'll send you a PR. It will produce this:
set_target_properties(leptonica PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include;${VCPKG_IMPORT_PREFIX}/include"
  INTERFACE_LINK_LIBRARIES "GIF::GIF;JPEG::JPEG;openjp2;PNG::PNG;TIFF::TIFF;WebP::libwebpmux;WebP::webp;ZLIB::ZLIB;m"
)

Note that CMake already changed the sequence of targets, based on transitive usage requirements.
I didn't change the linkage to PRIVATE because I don't know enought about leptonica. But I touched a few additional spots.

Proposed changes: https://github.com/LilyWangLL/vcpkg/pull/1

Proposed changes: LilyWangLL#1

get_filename_component(INCLUDE_DIR "${CMAKE_CURRENT_LIST_DIR}/../../include" ABSOLUTE) in patch cause tesseract build failed with the following error:
vcpkg\buildtrees\tesseract\src\4.1.1-87ae175ea6\src\ccmain\tesseractclass.h(30): fatal error C1083: Cannot open include file: 'allheaders.h': No such file or directory
So I removed it, keep original codes is set(Leptonica_INCLUDE_DIRS      "${CMAKE_CURRENT_LIST_DIR}/../../include;${CMAKE_CURRENT_LIST_DIR}/../../include/leptonica")

Thanks Lily!


Hotfix/crumbissuer

PR for #38

:+1:
It's maybe not a good practice to use e.printStackTrace?

@ojacquemart I checked the use of e.printStackTrace() in the code base and I found a coupled of them used in JenkindServer.java at the time, so I decided to use the method.
But I agree to your point. Do you have any good ideas for this? or someone does?

It's ok if it printStackTrace is already used in the codebase.

Speaking as an occasional user, why wouldn't we want printStackTrace(), @ojacquemart ?

By using a logging framework, you can log in different levels (trace, debug, info, warn, error, ...). Also, you can choose your output: file, console, ... and you can keep consistency in your logs format and so on. All that is often configurable through a file.
By the way, the PR is great and I am waiting for a merge :)

Yup, I've been waiting for getting this PR merged for more than a year ;)

First thanks for the PR, but can you please squash the commits...

ping @hamaron  :)

@ojacquemart Oops! I created another PR: #116 :)


Update Windows-Install-Binary.rst

Updated warning message: Clarified that the warning does not indicate issues with ROS2 installation, addressing potential confusion for beginners.

Sorry, I pasted the sentence at wrong location.
The correct location was meant to be at line 51 after the text:

It is normal that the previous command, if nothing else went wrong, outputs "The system cannot find the path specified." exactly once.

So, this sentence becomes:

It is normal that the previous command, if nothing else went wrong, outputs "The system cannot find the path specified." exactly once. If you encounter a warning message similar to [rti_context_adds_cmake_module], feel free to ignore it. This warning does not indicate any issues with the ROS2 installation functionality.

Since there has been no response here in over 2 weeks, I'm going to close this out.  Please feel free to respond to my latest comments and reopen.


Also inline non-concat coords

@rsignell-usgs - appears to do the right thing for time as well as valid_time for HRRR.

I added the fix from #75 as a temporary measure until the changes @rabernat mentioned in #70 are implemented


Query about ibkr login and logout

Is user login and logout on ibkr accomplished through 'start' and 'stop' of the relevant user container ??
I am using 1 container for each user of my system ...

Hey @raj-crec thanks for your question 👍
To start with - IBeam wasn't built with a functionality of logging users out in mind. It was meant to log the user in and keep it logged in.
However, in practice yeah, starting the container can be thought of as 'logging in', and stopping the container as 'logging out'.
Hope that helps!

Yup it does .. thanks for the clarification !


Initial CircleCI integration

We're migrating our CIs to CircleCI as initiated by @nhooyr. The only thing missing here is upload to GitHub Releases.

Closing in Favor of Travis.


OpenCL.exe

Hello. Can you share the OpenCL compilation?

Its here....
https://github.com/iceland2k14/AltCrack/blob/master/x64/Release/clAltCrack.exe

Its here....
https://github.com/iceland2k14/AltCrack/blob/master/x64/Release/clAltCrack.exe

Thanks 💯


cmd/compile: takes too much cpu and fails when compiling huge go files

I use this go-embed tool for embedding resources into my executable directly. It produces a file like this https://github.com/pyros2097/go-embed/blob/master/examples/assets/main_prod.go.
But When I use this to build my frontend images which approximates to 20MB into a single main.go file which is also 20MB but is 300K+ lines of code It takes forever to compile and then crashes my CPU. I guess this is a problem because I have only 4GB of memory and an core i3 processor.
But it seems to compile properly on my friends 16GB laptop core i7.
Is there anyway I can make it compile on my laptop? Maybe by splitting the single file into many files or something?
My GoENV and go version go1.5.2 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/pyros2097/Code"
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

Well I solved this in my tool,
previously the file structure was like this
package assets

// ImagesDocTXT file
var ImagesDocTXT = []byte{
	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x0a, 0x49,
	0x2d, 0x2e, 0xc9, 0xcc, 0x4b, 0x57, 0x30, 0x54, 0x30, 0x52, 0x30, 0xe6,
	0x02, 0x04, 0x00, 0x00, 0xff, 0xff, 0x23, 0xc2, 0x19, 0x35, 0x0e, 0x00,
	0x00, 0x00,
}

And so I thought the lexer has to read a lot of tokens for upto 300K loc and that might be clogging it up so I then rewrote all the data to this
package assets

// ImagesDocTXT file
var ImagesDocTXT = []byte("\x1f\x8b\x08\x00\x00\x09\x6e\x88\x02\xff\x0a\x49\x2d\x2e\xc9\xcc\x4b\x57\x30\x54\x30\x52\x30\xe6\x02\x04\x00\x00\xff\xff\x23\xc2\x19\x35\x0e\x00\x00\x00")

And now the no of lines drop to about 50 and everything works fine.
Well I guess its about how you generate large code bases and mostly dealing with code generators and not the go compiler.
Closing this one for now

I'm reopening this. Even if you worked around it, the issue remains.

iirc, there's a very old issue relating to very large slice literals causing the compiler to use lots of memory and slow down. What that ever fixed?

If you're referring to #11612 no, it's not fixed yet. It's in the 1.7 milestone.

@ALTree much older (known from at least around the 1.0-1.1 days). I'll try to dig it up. I suspect these are all the same issue though, and that fixing one will fix them all.

@extemporalgenome #6643 and d227d680ece216603c31e36ee995b814259325dc

@dominikh that's the one, thanks!

I'm experiencing some version of this as well. Working on a Go port of HTTPS Everywhere, and the build fails if I try to embed all the HTTPS Everywhere rules here:
https://github.com/getlantern/httpseverywhere/tree/too-large-for-go
Cutting down the size of the targets.go file by about half allows the build to complete, but otherwise just running go build with either 1.7.5 or 1.8 on darwin amd64 gives:
[httpseverywhere (master *)]$ go build
# github.com/getlantern/httpseverywhere
panic: runtime error: makeslice: len out of range

goroutine 1 [running]:
cmd/compile/internal/gc.newliveness(0xc4272221b0, 0xc42011c238, 0xc49bc7e000, 0x4a651, 0x5ac00, 0xc4da1da000, 0x832c, 0x9800, 0xc48ef5f898)
	/usr/local/go/src/cmd/compile/internal/gc/plive.go:717 +0x145

cmd/compile/internal/gc.liveness(0xc4272221b0, 0xc42011c238, 0xc4c21321c0, 0xc4c2132230)
	/usr/local/go/src/cmd/compile/internal/gc/plive.go:1740 +0x14f
cmd/compile/internal/gc.genssa(0xc45d30b440, 0xc42011c238, 0xc4c21321c0, 0xc4c2132230)
	/usr/local/go/src/cmd/compile/internal/gc/ssa.go:4500 +0x77c
cmd/compile/internal/gc.compile(0xc4272221b0)
	/usr/local/go/src/cmd/compile/internal/gc/pgen.go:443 +0x707
cmd/compile/internal/gc.funccompile(0xc4272221b0)
	/usr/local/go/src/cmd/compile/internal/gc/dcl.go:1292 +0xdc
cmd/compile/internal/gc.fninit(0xc420115c00, 0x3b, 0x70)
	/usr/local/go/src/cmd/compile/internal/gc/init.go:164 +0xb8d
cmd/compile/internal/gc.Main()
	/usr/local/go/src/cmd/compile/internal/gc/main.go:471 +0x23b1
main.main()
	/usr/local/go/src/cmd/compile/main.go:50 +0xfe

@myleshorton please file a new issue and cc me.

I think the original issue here is a straightforward dup of #6643. The other conversations have either been resolved or have their own issues. Closing.


Multiplatform project

Only referencing the .net standard is enough for this library to be
cross platform, however that means that when using this library, the
whole implementing framework has to be downloaded. By targeting
different platforms we avoid this problem.
The download is pretty significant, about 200-300MB.

This is only useful during development. Most of people use nuget library and better if this stay as .net standard.
@axgdev Hope Polly fulfill you needs :)


Remove White Spaces from the JSP files

This is a redo of Previous CR : https://github.com/apache/tomcat/pull/331
These changes enable the compiler to remove excess white space from the JSP files & thus reduce the JVM metadata
(Constant whitespace in a JSP is passed unchanged to the client browser (indentations, newlines, etc.).  This results in bloated constant strings, and general waste in I/O operations. Trimming this whitespace results in smaller constants).
This can be controlled by providing context init params inside web.xml file. Example attached.
Based on your previous suggestions the following changes have been made:

Pre tags will be left untouched - to protect the behavioral changes on that tag.
By default this remains false, and thus will not affect any other supporting feature (SMAP for instance as suggested in previous CR) .
The flag is now initialized as a JSP init param as mentioned in the example below.
PR is against master.

Apologies in case something is missed out.
If any official documentation is required, can you attach the links on the PR?

I've been wondering why this flag is set by a context param instead of along with all other jsp compiler flags in the jasper options?

@rotty3000 Thanks for the comment.
I have added the flag inside the jasper options as mentioned by you. Do we need any other change to this PR?

@markt-asf @rotty3000 Thanks for your time on this : )
I have marked all of the conversations as resolved after the latest commits. Do we have any more feedback on this PR?

@martin-g This PR is also sitting stagnant since a few days now although all comments have been resolved. Can you provide some update on this too ?

Before this PR goes any further I would like to see an example of a JSP that exhibits the problem that this PR is intended to solve.

The primary aim of this PR is to trim off the white spaces. I can give you an example here.
Optimized
  |
-- | --
  |
  | <title>Quick Servlet Demo</title>
  |
  |
  | Tomcat 9 Test app
  |
This has over 1000 Jar Files

  |
FastStartup Flag: true

  |
OptimizeJSPs Flag: true

  |
To see server Logs: Click Here

  |
To see Optimized Render times of tags: Click Here

  |
  |

Unoptimized:

--
  |
  |         <title>Quick Servlet Demo</title>
  |
  |
  |         Tomcat 9 Test app
  |
This has over 1000 Jar Files

  |
  |
FastStartup Flag: false

  |
OptimizeJSPs Flag: false

  |
To see server Logs: Click Here

  |
To see Optimized Render times of tags: Click Here

  |
  |
  |
  |
  |
  |
  |
  |

For large JSP files these white spaces are tremendous and also useless when passed over the network. The white spaces are good for readability for the user, and are not necessarily needed as source-compiled files.
Removing these white spaces result in fewer writes to internal buffers and fewer flushes to the network and thus results in lower latency.

That is an example of output.  Place data into a div to escape characters, div is later removed -->
        <c:if test="${not empty jspError}">

${jspError}

        </c:if>
        <c:catch>
            <c:if test="${not empty module.moduleError}">

${module.moduleError}

            </c:if>
        </c:catch>

${requestScope[feature.featureName]}

    </c:if>
    <c:if test="${not empty feature.containerId || feature.type == 'CONTAINER'}">

    </c:if>
    <c:if test="${includeDebug}">

        <%@ include file="/WEB-INF/views/jsp/features/debug/includeIsDebugJs.jsp" %>
    </c:if>

</c:if>

This is from a real file in a critical application but we stripped(or even marked ***) out certain proprietary lines.  If there is an error or such that's why.
The indented tags such as line 12  results in white space on the html output.  This white space is eliminated via this change.
Our application is filled with neatly-formatted JSPs that output waste such as that.   Place data into a div to escape characters, div is later removed -->
        <c:if test="${not empty jspError}">

${jspError}

        </c:if>
        <c:catch>
            <c:if test="${not empty module.moduleError}">

${module.moduleError}

            </c:if>
        </c:catch>

${requestScope[feature.featureName]}

    </c:if>
    <c:if test="${not empty feature.containerId || feature.type == 'CONTAINER'}">

    </c:if>
    <c:if test="${includeDebug}">

        <%@ include file="/WEB-INF/views/jsp/features/debug/includeIsDebugJs.jsp" %>
    </c:if>

</c:if>

This is from a real file in a critical application but we stripped(or even marked ***) out certain proprietary lines.  If there is an error or such that's why.
The indented tags such as line 12  results in white space on the html output.  This white space is eliminated via this change.
Our application is filled with neatly-formatted JSPs that output waste such as that.  One public-facing HTML page became 20% smaller after implementing this.

Have you considered using gzip compression for such big responses?
It'd help much better that the white space stripping.
 Place data into a div to escape characters, div is later removed -->
    <c:if test="${not empty jspError}">

${jspError}

    </c:if>
    <c:catch>
        <c:if test="${not empty module.moduleError}">

${module.moduleError}

        </c:if>
    </c:catch>

${requestScope[feature.featureName]}

</c:if>
<c:if test="${not empty feature.containerId || feature.type == 'CONTAINER'}">

</c:if>
<c:if test="${includeDebug}">

    <%@ include file="/WEB-INF/views/jsp/features/debug/includeIsDebugJs.jsp" %>
</c:if>

</c:if>

This is from a real file in a critical application but we
stripped(or even marked ***) out certain proprietary lines. If there is an
error or such that's why.
The indented tags such as line 12 results in white space on the
html output. This white space is eliminated via this change.
Our application is filled with neatly-formatted JSPs that output
waste such as that. One public-facing HTML page became 20% smaller after
implementing this.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apache/tomcat/pull/351#issuecomment-694997270, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AABYUQSCFOPFIU2VDW3NTNDSGOLNRANCNFSM4QTRLR2Q
.

Have you considered using gzip compression for such big responses? It'd help much better that the white space stripping.
…
 This is from a real file in a critical application but we stripped(or even marked ***) out certain proprietary lines. If there is an error or such that's why. 2. The indented tags such as line 12 results in white space on the html output. This white space is eliminated via this change. 3. Our application is filled with neatly-formatted JSPs that output waste such as that. One public-facing HTML page became 20% smaller after implementing this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#351 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABYUQSCFOPFIU2VDW3NTNDSGOLNRANCNFSM4QTRLR2Q .

GZIP compression is good for interleaved whitespace (90+%), however good is neither free nor complete (100%).

Playing the devil's advocate, here, for a moment:

gzip is not a solution, here, because it "hides" the problem instead of solving it. If the whitespace is unwanted, then remove the whitespace. You can then gzip and get even better compression!
This is an issue for any source code where source-indentation is valuable for the programmer/debugger, but useless in the output. I have the same problem with Velocity templates. Any structure which looks like this produces hude amounts of useless whitespace in the output:

foreach(thing) {
for(thing.otherThing) {
if(foo) {
whatever
} else {
something else
}
}
}
What you'd love to have in the output is a series of these:
        whatever
        something else
        whatever
        something else
        something else
        whatever
        something else
        something else

But instead, you get all of the above plus the indentation from each of those foreach/if/else/close-brace lines which are here to make sure the programmer doesn't go crazy.
So there is something to be said for the removal of the whitespace. I'm not 100% convinced this is the right approach, but the problem definitely exists and it would be nice to have a solution.

Taking the JSP sample above, I made the following changes so it would compile:

removed includes
replaced invalid expressions with hard-coded strings

The result was a blank page, 35 lines long with a total content of 178 bytes. Each line had an indent of a 0, 4 or 8 space characters.
Once I enabled the existing trimSpaces option for the JSP the output's content length dropped to zero.
 Place data into a div to escape characters, div is later removed -->
        <c:if test="${not empty jspError}">

${jspError}

        </c:if>
        <c:catch>
            <c:if test="${not empty module.moduleError}">

${module.moduleError}

            </c:if>
        </c:catch>

${requestScope[feature.featureName]}

    </c:if>
    <c:if test="${not empty feature.containerId || feature.type == 'CONTAINER'}">

    </c:if>
    <c:if test="${includeDebug}">

    </c:if>

</c:if>

@markt-asf is this the jsp after the modifications? If not, can you attach your modified file ?
Also, I know trimSpaces touches the empty output files/blank pages, but had there been any data in that it wouldn't have touched it...
There is a thread in this PR, I just tagged you in that.
TrimSpaces only solves particular cases. This parameter solves all that trimSpaces solves plus the extra things added on top of those.

Yes, that is the JSP after the modifications. I asked you for a JSP that demonstrated the utility of the feature you propose. The JSP you provided did not compile. I made the minimum changes required so it did compile. Once I had a JSP that compiled I tested it with the existing trimSpaces feature. It removed all the white space. Based on the evidence you have provided so far there is no need for the feature you propose.
If you want to continue to make the case for this feature then you need to provide a valid JSP (without further edits) that demonstrates this feature. If it helps to have access to tag libraries then you could write the JSP for the examples web application.

@markt-asf
I am sure you wouldn't be able to test this on proprietary code, because that will generate empty outputs/compile-errors after stripping out the important lines.
I am attaching the JSP from the tomcat code itself, and the results will be quite evident once you use the flag in this PR.
This is the index.jsp file from the ROOT web application in tomcat's code. You can use both the parameters and see the difference they exhibit. Also, do make sure to clear out the work folder after testing any of the parameter just to clear out the cache.
<%--
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements.  See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License.  You may obtain a copy of the License at

    http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
--%>
<%@ page session="false" pageEncoding="UTF-8" contentType="text/html; charset=UTF-8" %>
<%
java.text.SimpleDateFormat sdf = new java.text.SimpleDateFormat("yyyy");
request.setAttribute("year", sdf.format(new java.util.Date()));
request.setAttribute("tomcatUrl", "https://tomcat.apache.org/");
request.setAttribute("tomcatDocUrl", "/docs/");
request.setAttribute("tomcatExamplesUrl", "/examples/");
%>
<!DOCTYPE html>

        <meta charset="UTF-8" />
        <title><%=request.getServletContext().getServerInfo() %></title>
        <link href="favicon.ico" rel="icon" type="image/x-icon" />
        <link href="favicon.ico" rel="shortcut icon" type="image/x-icon" />
        <link href="tomcat.css" rel="stylesheet" type="text/css" />

                Home
                Documentation
                Configuration
                Examples
                Wiki
                Mailing Lists
                Find Help

${pageContext.servletContext.serverInfo}

If you're seeing this, you've successfully installed Tomcat.
                        Users are defined in:

$CATALINA_HOME/conf/tomcat-users.xml

In Tomcat 10.0 access to the manager application is split between
                           different users.   Read more...

Release Notes

Changelog

Migration Guide

Security Notices

Documentation

Tomcat 10.0 Documentation

Tomcat 10.0 Configuration

Tomcat Wiki

Find additional important configuration information in:

$CATALINA_HOME/RUNNING.txt

Developers may be interested in:

Tomcat 10.0 Bug Database

Tomcat 10.0 JavaDocs

Tomcat 10.0 Git Repository at GitHub

Getting Help

FAQ and Mailing Lists

The following mailing lists are available:

tomcat-announce

                                Important announcements, releases, security vulnerability notifications. (Low volume).

tomcat-users

                                User support and discussion

taglibs-user

                                User support and discussion for Apache Taglibs

tomcat-dev

                                Development mailing list, including commit messages

Other Downloads

Tomcat Connectors

Tomcat Native

Taglibs

Deployer

Other Documentation

Tomcat Connectors

mod_jk Documentation

Tomcat Native

Deployer

Get Involved

Overview

Source Repositories

Mailing Lists

Wiki

Miscellaneous

Contact

Legal

Sponsorship

Thanks

Apache Software Foundation

Who We Are

Heritage

Apache Home

Resources

Copyright ©1999-${year} Apache Software Foundation.  All Rights Reserved

I have been doing some simple tests to compare the behaviour of compression with and without this PR. Using the default index page as an example, gzip achieves 80.1% compression. Applying this PR and then gzip achieves 80.8% compression. Given the nature of gzip compression and the optimisations provided by this PR I'd expect to see similar results for other input. i.e. gzip provides significant compression and adding this PR makes only a minimal difference.
gzip does require additional CPU to perform the compression so I want to do some more experiments to understand the impact of this PR under various load scenarios.
Separately, I'll note that the PR as currently provided fails the Checkstyle checks.

I still think gzip is a red herring, here. If you remove whitespace prior to gzipping, you'll likely see that gzip gives you a lower compression ratio when comparing the stripped-text versus non-stripped-text against the final result. Comparing the original text to the stripped-text versus non-stripped text will be a better comparison.
But the main point is that removing whitespace itself is an optimization itself whether gzip is at play or not.

The relevance of gzip depends on what you are trying to optimize. If you are trying to minimise network bytes then gzip is very relevant. For the minimise network bytes use case, this PR + gzip offers very small gains compared to gzip alone. Hence, in my view, this PR is not supported by the minimise network bytes use case.
Where this PR does make a difference - at least in my tests - is throughput. The bottleneck on my system appears to be in the Tomcat and/or JVM network stack and is somewhere around 3Gbps. At least that is how it appears. Because this PR reduces the size of the response without (unlike gzip) adding delay or CPU overhead, it increases throughput in terms of requests per second. What I am unclear on is how likely this bottleneck is to appear in a real-world scenario.
Other thoughts that come to mind are:

how does this compare to an HTML minifier filter (I'd like to test this)
if we do apply this PR, does this need to be a separate configuration option or does it make more sense as an additional Tomcat specific extension of trimSpaces like "single" is. Maybe call it "extended".

If I may bring one point; I wonder if maybe this optimization is later in the process than would really benefit even more concerns.
For instance, if the optimization was made in jasper compiler itself, then you also lower the method line counts by reducing the number of write statements which reduces the likeliness of hitting the 65535 method size limit.
To summarize, you could have 3 levels of improvement from making the change earlier in the lifecycle of JSPs:

reduce the compilation size limit scenarios
execute less statements
write less bytes

Just a thought.

@rotty3000 This is what I was thinking would be better than post-processing. The JSP->class translation should be a one-time hit rather than a filtering-step that requires CPU time and possibly more buffering for each response.
@markt-asf Reducing useless whitespace and compression are both ways of reducing the overall bulk of a response. In one case (whitespace) you are removing literally useless data. In the other case, you are retaining all the useful data but reducing the size. The two together are better than either one alone IMO. But comparing the two directly is a false equivalence: it's not fair to say "you get a bigger benefit from using gzip vs. whitespace trimming" because they are different things. Not trying to cause a big fight; just mentioning that they are both pieces of a larger puzzle.
Finally, a post-processing HTML-reducing filter would be useful for situations where JSP isn't being used at all and might be a superior solution merely due to its flexibility. There are those performance considerations, though, of course. But we are talking about a trade-off between (essentially) CPU usage and network bulk, so there will always be a trade-off.

Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate.
It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces.
So the example above:
|
|         <title>Quick Servlet Demo</title>
|

won't produce:
|
| <title>Quick Servlet Demo</title>
|

In addition: why preserve 1 new line ? HTML minifiers would produce something like ...<title>...</title>...

Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate.
It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces.
So the example above:
|
|         <title>Quick Servlet Demo</title>
|

won't produce:
|
| <title>Quick Servlet Demo</title>
|

In addition: why preserve 1 new line ? HTML minifiers would produce something like ...<title>...</title>...

I think the node text comes like this \n<indentations> and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to \n after the processing. I assume you are not taking the node correctly.
But on a broader aspect
|
|         <title>Quick Servlet Demo</title>
|

Will produce
|
|     <title>Quick Servlet Demo</title>
|

Since there is no new test for this I am not sure but looking at the code I think "Remove white space" is not quite accurate.
It is rather "Remove empty lines". Because BLANK_LINE_PATTERN would match only if there zero or more empty spaces, followed by at least one new line, followed by 0 or more empty spaces.
So the example above:
|
|         <title>Quick Servlet Demo</title>
|

won't produce:
|
| <title>Quick Servlet Demo</title>
|

In addition: why preserve 1 new line ? HTML minifiers would produce something like ...<title>...</title>...

I think the node text comes like this \n<indentations> and this will be caught by the BLANK_LINE_PATTERN and thus will later be turned down to \n after the processing. I assume you are not taking the node correctly.
But on a broader aspect
|
|         <title>Quick Servlet Demo</title>
|

Will produce
|
|     <title>Quick Servlet Demo</title>
|

I agree to the minifying part, so we can rather just replace it with an empty character in the end instead of new line character and that will do the job too and will thus be more optimized.

I'd recommend adding a test to cover this use-case.

@markt-asf @ChristopherSchultz @rmaucher Do we have updates on this issue?

@markt-asf @ChristopherSchultz @rmaucher Do we have updates on this issue?

I've added this to 10.0.x and 9.0.x with a few changes. The notable changes were:

configured via a new trimSpaces value of extended rather than a new option
also removed leading and trailing whitespace from template text
tests check behaviour with and without configuration option enabled
formatting tweaks


Use spam

Spams e basically

I thought u quitted hacking
@notgoodusename

I thought u quitted hacking
@notgoodusename

idk what to say i just like coming back to the cheating scene

why this feature exist ?

#619

Why a static function?

i don remember why i used the static function, but i think it should work the same without it

What is the purpose of this?

i use it to grab people gun when they throw it or troll people with doors with doors

Wouldn't it make bomb undefusable for you?

yeah didnt think of that maybe i can check if the bomb is planted and disable it

that should fix it

that should fix it

@notgoodusename More broken, even worse.

what do you mean "even worse"?

what do you mean "even worse"?

The "bomb fix" don't work due to "many command's" and get kicked.

"even worse"

Because before you fix it, don't have this "many command's" error and get kicked from server.

what do you mean "even worse"?

The "bomb fix" don't work due to "many command's" and get kicked (weird).

"even worse"

Because before you fix it, don't have this "many command's" error and get kicked from server.

Weird because it only sends actions not commands

what do you mean "even worse"?

The "bomb fix" don't work due to "many command's" and get kicked (weird).

"even worse"

Because before you fix it, don't have this "many command's" error and get kicked from server.

Weird because it only sends actions not commands

When I try to see if really get fixed on a localhost server with Bot's, I plant the bomb with "E" button and I get kicked by "To many command's" (I don't even test if works on matchmaking or something).

ohhh that makes sense

By the way, nice work with new Anti Aim improvement. I already had, 'cause the request before, now just needed to change and fix some thinks and is working fine, thanks.

By the way, nice work with new Anti Aim improvement. I already had, 'cause the request before, now just needed to change and fix some thinks and is working fine, thanks.

the fix to usespam is this
void Misc::usespam(UserCmd* cmd) noexcept
{
static bool usespam = true;
const auto localPlayer = interfaces.entityList->getEntity(interfaces.engine->getLocalPlayer());
if (localPlayer->isDefusing()) {
return;
}
if (!localPlayer->isAlive() || localPlayer->inBombZone()){
return;
}
if (config.misc.usespam && cmd->buttons & UserCmd::IN_USE) {
if (usespam)
{
cmd->buttons |= UserCmd::IN_USE;
usespam = false;
}
else
{
cmd->buttons &= ~UserCmd::IN_USE;
usespam = true;
}
}
}

By the way, nice work with new Anti Aim improvement. I already had, 'cause the request before, now just needed to change and fix some thinks and is working fine, thanks.

Thanks!


crash

when I am doing the optimization the tool crashes and does not let me continue with the process

You have to watch the powershell prompt before it will become responsive. Just a limitation of running in Powershell.
There are a couple forks utilizing runspaces or jobs that could alleviate this, but probably won't be implemented until next year.


[Textgrid for dataset]

I am creating textgrid files for my dataset. Can you guide me how to create that file? Or you can give me information. Thank you so much!

Hi @frank269 , I used Montreal Forced Aligner to create textgrid files. Visit https://montreal-forced-aligner.readthedocs.io/en/latest/ for more information.

thank for your response.

I used MFA 2.0 to align the text, but it through the error. How can i generate lexicon for vietnamese?
This is an error:
montreal_forced_aligner.exceptions.PronunciationAcousticMismatchError: There were phones in the dictionary that do not have acoustic models: a, e, i, u, y, ê, ề

The pretrained acoustic model does not include these phones, in this case, you have to train your own acoustic model. See https://montreal-forced-aligner.readthedocs.io/en/latest/aligning.html#align-using-only-the-data-set for more information.

I tried using the first 6 files and the lexicon file in infore data to align and train with the command:
./bin/mfa_train_and_align MFA/dataset MFA/lexicon.txt MFA/aligned
But it only has the first file that has the correct textgrid file, and the other files that give the wrong data, Where did I go wrong?

To train a MFA model, you need: a lexicon file, a wav/text data directory.
The wav/text data directory includes all your audio clips and the transcript files. Each A.wav clip requires a A.txt transcript file in the same directory.

Yes, I used the first 6 files and lexicon file in the database infore you provided, I also manually created 6 transcription files for each audio clips. But when I run command train, the output of the first file is correct, but the other files are wrong, here are the results of the first 6 files:
1.zip

It is possible that your dataset is too small, so MFA cannot learn a useful model from that little data.

This is a notebook that I used to align InfoRe data with a slightly different phoneme set
https://colab.research.google.com/gist/NTT123/95b12ca42a4bdd1a856aba0fbb0f8936/infore-mfa-tutorial.ipynb

Oh, thank you so much!

This is a notebook that I used to align InfoRe data with a slightly different phoneme set
https://colab.research.google.com/gist/NTT123/95b12ca42a4bdd1a856aba0fbb0f8936/infore-mfa-tutorial.ipynb

I'm using your colab script but I got issue:

I try to fix but I don't know what happend in MFA aligner. This is example of my lexicon textGrid and txt file

And this is my tree:

Thanks!


Is the dashboard open source, which we can monitor the process of running portia-generated code？

When I was running  the portia-generated code on scrapinghub , there is a dashboard which shows the projects and spiders and can monitor the state of them, so  could I get a dashboard set locally when I run portia on my own environment? Looking forward to any help.

You can try scrapyd but it is not as full featured as what you get on scrapinghub.com


Names non appearing in the CSV extracts

Hi @dhananjay1405 ,
The Utility creates CSVs pertaining to each table.
When I tried to check for data, I could only see 1s and 0s in majority of the columns.
PFA CSV sample files generated by the utility.
CSV Sample Files.zip
Please guide me on what I can do to see the actual data.
--
Regards
Saurabha J

Duplicate of #3


TF 1.15 to TF 2.0 breaking changes even with tf.compat.v1 for saving models for use with tf serving

Please make sure that this is a bug. As per our GitHub Policy, we only address code/doc bugs, performance issues, feature requests and build/installation issues on GitHub. tag:bug_template
System information

Have I written custom code (as opposed to using a stock example script provided in TensorFlow): yes
OS Platform and Distribution (e.g., Linux Ubuntu 16.04): debian dockerized
Mobile device (e.g. iPhone 8, Pixel 2, Samsung Galaxy) if the issue happens on mobile device: n/a
TensorFlow installed from (source or binary): docker
TensorFlow version (use command below): 1.15 and 2.0.0
Python version: 3.7
Bazel version (if compiling from source): n/a
GCC/Compiler version (if compiling from source): n/a
CUDA/cuDNN version: n/a
GPU model and memory: n/a

You can collect some of this information using our environment capture
script
You can also obtain the TensorFlow version with: 1. TF 1.0: python -c "import tensorflow as tf; print(tf.GIT_VERSION, tf.VERSION)" 2. TF 2.0: python -c "import tensorflow as tf; print(tf.version.GIT_VERSION, tf.version.VERSION)"
Describe the current behavior
Currently trying to export a tf.keras model in v 2.0.0 using tf.compat.v1 functions (which will be removed in the future) does not work. Using the same code in 1.15.0 works.  Currently there is not a tf.compat.v2 function which is concerning for those using this functionality for compatibility with tf serving.
Describe the expected behavior
That code with tf.compat.v1 works in 2.0 as it does in 1.15
Code to reproduce the issue
This repo: https://gitlab.com/SumNeuron/docker-nf
Is my dockerized TF MWE. It contains the code necessary to reproduce the issue as well as subsequent steps (e.g. spin up a dockerized web app with the served model).
In order to reproduce the bug there are two files of relevance:

Dockerfile.ai
notebooks/Make Toy Model.ipynb

The first two lines of Dockerfile.ai:
FROM tensorflow/tensorflow:1.15.0rc2-gpu-py3-jupyter
# FROM tensorflow/tensorflow:2.0.0-gpu-py3-jupyter

are for convenience of toggling between TF versions.
To spin up the service:
docker-compose -f docker-compose.ai.development.yml build
docker-compose -f docker-compose.ai.development.yml up

Then navigate to the mounted notebook.
After running the toy model there is a code cell with markdown content:

Simple Save for Serving

under which has the following code (which is more or less copy-paste from a tf official documentation example).
MODEL_DIR = '../models/serving/toy'
version = 1
export_path = os.path.join(MODEL_DIR, str(version))
if not os.path.isdir(export_path):
    os.makedirs(export_path)

tf.compat.v1.saved_model.simple_save(
    tf.compat.v1.keras.backend.get_session(),
    export_path,
    inputs={'input_tensor': model.input},
    outputs={'output_tensor': model.outputs[0]})

This will work when using tf version 1.15. It will fail with tf version 2.0
Other info / logs
Include any logs or source code that would be helpful to diagnose the problem. If including tracebacks, please include the full traceback. Large logs and files should be attached.

@SumNeuron , the notebook you have shared saves the model successfully. Please share a notebook in which you face the issue.

@nikochiko did you change the FROM statement in Dockerfile.ai from
FROM tensorflow/tensorflow:1.15.0rc2-gpu-py3-jupyter
# FROM tensorflow/tensorflow:2.0.0-gpu-py3-jupyter

to
# FROM tensorflow/tensorflow:1.15.0rc2-gpu-py3-jupyter
FROM tensorflow/tensorflow:2.0.0-gpu-py3-jupyter

@SumNeuron What error are you getting? You have provided too little information to help.
And to make TF1.X code compatible with TF2.X, you need to disable eager execution with tf.compat.v1.disable_eager_execution().
Works fine for me. See gist: https://colab.research.google.com/gist/nikochiko/cc43b4e4e57918669c217c9fe91270b1/34466.ipynb

@oanush @gowthamkpr , I think this issue is wrongly marked as a bug. Please see.

@nikochiko ah so it was
tf.compat.v1.disable_eager_execution()

that was the missing line?

@SumNeuron Yes. See the gist.

@nikochiko Then for now I recommend perhaps updating the error message.
Set enable_eager = True and run with 2.0 and get the error:
RuntimeError: build_tensor_info is not supported in Eager mode.

to include a statement pointing to tf.compat.v1.disable_eager_execution()

@SumNeuron Why did you change it to True?

@nikochiko to see the error message. When eager was rolled out there was emphasis on enabling it (auto-on for 2.0), but i have yet to encounter that it could or may need to be disabled for v1 compatibility, so adding a note to the error message may be useful to other users as well

No, look, you can avoid errors altogether with this syntax instead:
if tf.executing_eagerly():
  tf.compat.v1.disable_eager_execution()

@nikochiko the point I was trying to make is that the need for
tf.compat.v1.disable_eager_execution()

is not made explicitly clear, especially as many tf.compat.v1 functions also work in eagerly.
Semirelated, do you know the v2 version of the code (e.g. how to achieve the same .pb file without v1 functions?

@SumNeuron , if I understand correctly, you are trying to make your code compatible with both TensorFlow 1.X and TensorFlow 2.X. You should first decide whether you want to have eager execution enabled or not, and you can continue with that. As you mentioned that some v1 compat functions are compatible with eager execution, but it will be weird to have your code execute differently in 1.X and 2.X considering that your goal was to make it usable in both.
For the TensorFlow 2.X version of the code, you can use the tf_upgrade_v2 utility. It will convert all v1 symbols to their v2 equivalents.

@nikochiko Not quite. I am happy to transfer to v2 and live in the new TF paradigm.
You might have noticed in your colab (copy from the repo I provided), the header for the code which breaks without disabling eager, is "Simple Save for Serving".
The only thing I want to be compatible in V2 is saving a trained keras model for later use with tf-serving

@SumNeuron , if you are ready to move to TF2, you only need to apply tf_upgrade_v2.
@gowthamkpr , please take this on from here.

@SumNeuron I am gonna close this issue for now as its not related to bug/performance, build/install, feature or docs related issues. Please post any more questions that you have on stackoverflow as there is a wider community to respond. Thanks @SumNeuron for answering.


fixed #263: message:make creates 'e' and 'n' locale directories if locale is set to 'en'

Besides, this also fixes MO file creation during 'build' and 'serve'

any chance of merge to master soon?


Incorrect callback on cloneElement

Problem
We are cloning elements and overriding their methods without calling the original ones. e.g. if we have an element  and we clone that element the doSomething won't execute as the onMouseEnter has been completely replaced.
Solution
Call the original method i.e. instead of overwriting a method we extend it.
Where this occurs:

Hoverable (I tried fixing this but just found I didn't really do a thing)
Tooltip (same as Hoverable)
Form (This will be partly fixed once the new date picker is merged)

Assign me here

For better clarify, what we are fixing here is cases like this:
<Form>
    <TextInput ref={r => this.texInput = r} onBlur={this.callThePolice} />
</Form>

You may be expecting the ref this.texInput to be set and this.callThePolice to be called on blur but none of that will happen.

Yep, thanks!

Reviewing label has been removed, please complete the "BugZero Checklist".

The solution for this issue has been :rocket: deployed to production :rocket: in version 1.2.91-1 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/15990

If no regressions arise, payment will be issued on 2023-04-05. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

[ ] External issue reporter
[ ] Contributor that fixed the issue
[ ] Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty


doc: fix typos in NestJS docs

[x] Change "automatatic" to "automatic".
[x] Change "thru" to "through".

Thanks! The changes are made in the next docs only (docs/docs), which is fine, but if you'd like to see them in the current (or older) versions, you'd also have to fix it in the docs/versioned_docs folder.
Will leave it up to you :]


Nit-picky

Nice state-machine implementation!
A few questions which probably don't affect operation:

unsigned long last_read_ms; is only declared and assigned in the instance creation. It is never used. Should it be removed?

pin1 and pin2 are declared as int. Would byte be a better choice for space?

void ESPRotary::loop() contains
int s = state & 3;
Maybe make "s" a byte? It's range would appear to be limited to 0-15. Since s is always assigned to the byte variable "state", perhaps just use state and skip "s"?

(added with edit)

int steps_per_click: It would seem rather unlikely that this would be negative and that the value would be over 127 / 255. Make it a int8_t or uint8_t?

Save nine bytes of RAM!
The default for the ESPRotary constructor are int lower_bound = INT16_MIN, int upper_bound = INT16_MAX. INT16_MAX is negative 1 for an int. Was this your intention? Probably not much of an issue as most would would set their own bounds.
Thanks for making ESPRotary available!
Cheers -Jim

Hey Jim,
sorry for the late reply and thanks for the feedback.
Was weeding out things in some other Arduino code...
I removed last_read_ms, turned a few ints into bytes.
The upper / lower bounds were suggested in this issue.
I pushed these changes plus a few smaller ones as well. I will release them in a few days after I tested everything again.
Cheers
l.


Feature Request: Specify Github Token as Command-Line Arg

As a companion to Issue 182, there appears to be valid use cases for specifying a Github token via command-line argument.

--access-token= is an option in the current version. I believe this was added in v3.0