Add mypy to pre-commit and test pipeline

Proposed change

Last fixes for mypy. Thanks @cdce8p for your initial and tremendous help getting everything in!
When this is merged, we probably don't need #396 anymore, am I correct?
Type of change

[ ] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (which adds functionality to this library)
[ ] Breaking change (fix/feature causing existing functionality to break)
[x] Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:

Checklist

[x] The code change is tested and works locally.
[ ] Tests have been added to verify that the new code works.

Codecov Report

Merging #432 (6d55748) into master (fa4cb00) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #432   +/-   ##
=======================================
  Coverage   97.68%   97.68%
=======================================
  Files          16       16
  Lines        1207     1207
=======================================
  Hits         1179     1179
  Misses         28       28

Flag
Coverage Δ

3.10.x
97.59% <100.00%> (ø)

3.6
97.58% <100.00%> (ø)

3.7
?

3.8
?

3.9
97.61% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files
Coverage Δ

bimmer_connected/vehicle/remote_services.py
100.00% <ø> (ø)

bimmer_connected/vehicle/vehicle.py
99.31% <100.00%> (ø)

bimmer_connected/vehicle/vehicle_status.py
100.00% <100.00%> (ø)

Continue to review full report at Codecov.

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

Many thanks for your detailed suggestions and further reading. Happy to include them :)


Implement std::marker::Sized for service traits

Is it possible to implement std::marker::Sized for the service traits like S3? I'm trying to pass my S3 instance around from a function like get_client and I'm getting the error.
error[E0277]: the trait bound `rusoto_s3::S3 + 'static: std::marker::Sized` is not satisfied
   --> src/directory/s3_directory.rs:101:29
    |
101 |     fn get_client(&self) -> S3 {
    |                             ^^ `rusoto_s3::S3 + 'static` does not have a constant size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `rusoto_s3::S3 + 'static`
    = note: the return type of a function must have a statically known size

Maybe I just need a lifetime?

I needed to return Box<S3> and now all is well.


Use "web_extra_daemons"

This PR upgrades this addon to use "web_extra_daemons" to start the queue worker.
This has the benefits of

simplifying config
does not break container if supervisor crashes

Because in DDEV v1.23.0, web_extra_daemons failing don't bring down the container:

Consider adding a .ddev/config.ddev-laravel-queue.yaml with ddev_version_constraint: v1.23.0, which will make sure that  people get this behavior
Consider wrapping the queue runner with a while true; do <run queue> || true; done so that the queue exiting doesn't end up with no runner.

And thanks!

This implementation removes some files. Do we need to add something that removes them or is that handled automatically?

To remove files, you probably want to do it with the pre_install_actions, post_install_actions, or removal_actions, all of which take bash commands. Make sure you don't remove files that don't have #ddev-generated in them.
if grep "#ddev-generated" <file>; then rm <file>; fi

Testing with
ddev get https://github.com/tyler36/ddev-laravel-queue/tarball/10-Consider-migrating-to-web_extra_daemons

When I install I see
Executing post-install actions:
grep: web-build/Dockerfile.ddev-laravel-worker: No such file or directory

grep: web-build/laravel-worker.conf: No such file or directory

The grep should have 2>/dev/null on it probably.

The grep should have 2>/dev/null on it probably.

Updated.


only with ollama "invalid escape sequences in the input string" when the data is having "_ "

# Ollama
        self.custom_llm = ChatOpenAI(
            model="mistral",
            #model="openhermes",
            base_url="http://localhost:11434/v1",
            api_key="ollama",  # something random
            temperature=0,
        )

main.py
def run():
    inURL = input("🚀 Enter YouTube URL: ")
    video_id = extract_video_id(inURL)
    # video_id = extract_video_id(input("🚀 Enter YouTube URL: "))
    if not video_id:
        print("🚨 Invalid YouTube URL provided.")
        return

    inputs = {"video_id": video_id, "url": inURL}
    crew = YoutubeCommentsCrew()
    result = crew.crew().kickoff(inputs=inputs)
    print("Analysis Result:")
    print(result)'

when the url passed is https://www.youtube.com/watch?v=sNa_uiqSlJo therefore video_id is sNa_uiqSlJo
the resulting verbose
Entering new CrewAgentExecutor chain...
 I need to retrieve all comments for the specified YouTube video ID "sNa\_uiqSlJo" using the YouTube Comments Fetcher tool.

Action: YouTube Comments Fetcher
Action Input: {"video\_id": "sNa\_uiqSlJo"}<unknown>:1: SyntaxWarning: invalid escape sequence '\_'
<unknown>:1: SyntaxWarning: invalid escape sequence '\_'

modified the main.py with  video_id = str (video_id) but still does not work
def run():
    inURL = input("🚀 Enter YouTube URL: ")
    video_id = extract_video_id(inURL)
    # video_id = extract_video_id(input("🚀 Enter YouTube URL: "))
    if not video_id:
        print("🚨 Invalid YouTube URL provided.")
        return

    # need to pass the video_id with _ without the escape ie sNa_uiqSlJo instead of sNa\_uiqSlJo
    # video_id = str (video_id)
    ## does not work

    inputs = {"video_id": video_id, "url": inURL}
    crew = YoutubeCommentsCrew()
    result = crew.crew().kickoff(inputs=inputs)
    print("Analysis Result:")
    print(result) #.encode("utf-8"))

fixes the issue of  "sNa_uiqSlJo"
Entering new CrewAgentExecutor chain...
 I need to retrieve all comments for the specified YouTube video ID "sNa_uiqSlJo" using the YouTube Comments Fetcher tool.

Action: YouTube Comments Fetcher
Action Input: {"video_id": "sNa_uiqSlJo"}

I am a newbie into AI, so dont know if this is bug or feature..

That's an old issue. (Dumb) LLMs seem to always think you have to escape underscores. Same if a tool is for example called "web_search". You can get around this by just writing python code that deletes all "" in the video_id. Then it doesn't matter if the model is dumb.


Fix accounting for a thread's shared lock state

Change-Id: I7e9bfc77ddcd3f972468e59a38779dd5523f9412

CI gfxreconstruct build queued with queue ID 37491.

CI gfxreconstruct build # 2234 running.

CI gfxreconstruct build # 2234 passed.

I'm afraid that this patch needs some work. I just run into an assertion failure in https://github.com/LunarG/gfxreconstruct/pull/910/files#diff-86d287f9befc06931be97da47215184aaf84c64f41e94d12893623de00d5e833R36

I can test this later but as noted in review this misses one of the point mentioned in #910. There's also the question whether we should retain the upgrading functionality at all (also mentioned in review comment).

Just had the time to check MSVC's shared_mutex implementation and it seems at least recent versions use SRWLOCK, which should not require kernel roundtrip unless contended.
It might be interesting to redo the performance test David has done here and check if it's still worth rolling our own sync primitive.

@ishitatsuyuki Thanks for looking into this. Using std::shared_mutex appears to have similar capture-time performance as the internal SharedMutex implementation. I'll look into switching GFXR to use it.

#979 changes GFXR to use std::shared_mutex. Once that is merged I will close this PR.


Passive Scalar Issue In GR

Hi all,
It looks to me like there might be an error in the eos_scalars.cpp functions when using relativity.  Since the conservation equation is
d_t (gdet rho u^t r) + d_i (gdet rho u^i r) = 0
shouldn't the conservative scalar s be [s = density * u^t  * r] instead of just [s = density * r] (that is, there is a missing factor of the time component of the contravariant four-velocity)?
I could be missing something in the code.
Sean

This is a known bug in the development version. A fix was developed 2019-09-16, but is on hold indefinitely because of other changes due to be made. Hopefully it will be included in the next public release. Until then, contact me and I can figure out how to patch the public version.

Here is the fix for anyone who might be interested. It is applied on top of commit d5350951 from 2019-10-15.

Download scalar_fix.patch (rename it to remove .txt) into the top of the Athena directory.
git apply scalar_fix.patch
cd tst/regression
./run_tests.py scalars/sr_hydro_scalars

If the test passes, the fix should be working. It should take less than a minute to run.
Note: this will create tst/regression/scripts/tests/scalars/sr_hydro_scalars.py, and it will modify
inputs/hydro_sr/athinput.mb_1
inputs/hydro_sr/athinput.mb_2
inputs/hydro_sr/athinput.mb_3
inputs/hydro_sr/athinput.mb_4
src/athena.hpp
src/bvals/bvals.cpp
src/bvals/bvals_refine.cpp
src/eos/eos.hpp
src/eos/eos_scalars.cpp
src/hydro/srcterms/hydro_srcterms.cpp
src/hydro/srcterms/hydro_srcterms.hpp
src/mesh/mesh.cpp
src/pgen/gr_shock_tube.cpp
src/pgen/strat.cpp
src/task_list/time_integrator.cpp


spamassassin: add smoke-test

Description of changes
This adds a quick smoke-test to the SpamAssassin derivation.
Note that the test currently fails expectedly, since SpamAssassin generates and compiles C code that makes use of crypt.h, which the SpamAssassin package currently does not have access to (cf. #215463).

Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[x] Tested, as applicable:

NixOS test(s) (look inside nixos/tests)
and/or package tests
or, for functions and "core" functionality, tests in lib/tests or pkgs/test
made sure NixOS tests are linked to the relevant packages

[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
[ ] Tested basic functionality of all binary files (usually in ./result/bin/)
23.05 Release Notes (or backporting 22.11 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module
[ ] (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes

[ ] Fits CONTRIBUTING.md.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/prs-ready-for-review/3032/1899

Before we merge this it should work and the error should be fixed.

Before we merge this it should work and the error should be fixed.

Do failing package tests block CI somehow?

No but they should and the next person updating or touching spamassassin will notice that ofborg fails and will be confused. Please fix the test first and then I'll happily merge it.

No but they should and the next person updating or touching spamassassin will notice that ofborg fails and will be confused.

Hmm, strictly speaking, the test is working and does indicate the state of the package, since Spamassassin is currently broken in nixpkgs. But it’s fine by me if we get it fixed first.

Please fix the test first and then I'll happily merge it.

The test itself is working, only Spamassassin is currently broken as per #215463. I’ll try and take another look at that issue, although I’m no big on C and its tooling, unfortunately. Alternatively, we might want to mark Spamassassin as broken.


Issue with DDBoost backup -- whitespace in output file

**Description of the issue **
Hello, we had an issue when using your script in conjunction with the DDBoost backup software. the output file normally around 160Kb was now 5540Kb and it was filled with whitespace and ---- symbols. it still had all the other info but readablilty was low. this also sometimes made the log backups fail. We were provided an adaptation to your script from DELL to remove these whitespaces and symbols and since then we did not encounter any failed LOG backups
SQL Server version and edition
Execute SELECT @@VERSION
Microsoft SQL Server 2019 (RTM-CU16-GDR) (KB5014353) - 15.0.4236.7 (X64)   May 29 2022 15:55:47   Copyright (C) 2019 Microsoft Corporation  Enterprise Edition (64-bit) on Windows Server 2022 Datacenter 10.0  (Build 20348: ) (Hypervisor)
Version of the script
Version: 2022-12-03 17:23:44 (i downladed the latest version to check)
What command are you executing?
EXECUTE [dbo].[DatabaseBackup]
@Databases = 'ALL_DATABASES',
@BackupType = 'LOG',  -- OR 'LOG',
@BACKUPSOFTWARE = 'DATA_DOMAIN_BOOST',
@DatadomainBoostHost= 'dd64001b.central.fluxys.int',
@datadomainboostuser= 'ddboost_sql2022',
@datadomainBoostDevicePath = '/DD_SQL_PRD',
@DataDomainBoostLockboxPath = 'E:\Program Files\DPSAPPS\common\lockbox',
@CleanupTime = 168, -- or Lower or higher value (168 for log)
@LogToTable = 'Y',
@CheckSum = 'Y',
@Compress = 'N',
@BufferCount = 500
What output are you getting?
it is succesfull but sometimes not.
Dell told us to adapt this line
SET @CurrentCommand = 'DECLARE @ReturnCode int EXECUTE @ReturnCode = dbo.emc_run_backup '''
DatabaseBackup_LOG_0x33D4DF2723FF8A44AD6D559111DD0EB8_1_20230220_85923.txt
DatabaseBackup_LOG_0x33D4DF2723FF8A44AD6D559111DD0EB8_1_20230220_84500.txt
to
SET @CurrentCommand = 'declare @t table (msg nvarchar(MAX)); DECLARE @ReturnCode int; insert into @t (msg) EXECUTE @ReturnCode = dbo.emc_run_backup '''
i tested this and the whitespaces are gone this way, we did not have any issues running it for over 72 hours.
I've added the output file from before the change and after the change.
i'm now implementing this change across all out instances. Since the script keeps working i hope this can be implemented in the official version to maybe help users with similar cases.
Have a nice Day
Joris

This sounds like a bug in emc_run_backup. Is this still an issue with the latest version of Data Domain Boost?


brew upgrade Error on macOS 10.15 Developer Beta7

##Here is the msg:
miyamoto@macbookpro ~ % brew upgrade
Warning: You are using macOS 10.15.
We do not provide support for this pre-release version.
You will encounter build failures with some formulae.
Please create pull requests instead of asking for help on Homebrew's GitHub,
Discourse, Twitter or IRC. You are responsible for resolving any issues you
experience, as you are running this pre-release version.

==> Upgrading 6 outdated packages:
vim 8.1.1600 -> 8.1.1700, cmake 3.14.5 -> 3.14.6, python 3.7.3 -> 3.7.4, glib 2.60.4_1 -> 2.60.5, rust 1.35.0 -> 1.36.0, node 12.5.0 -> 12.6.0
==> Upgrading vim
Error: The following formula
  [#<Dependency: "python" []>, #<Options: []>]
cannot be installed as binary package and must be built from source.
Install the Command Line Tools:
  xcode-select --install

miyamoto@macbookpro ~ %   xcode-select --install
xcode-select: error: command line tools are already installed, use "Software Update" to install updates

I'm having the same issue on 10.15 beta.
==> Upgrading ffmpeg
Error: The following formula
  [#<Dependency: "python" []>, #<Options: []>]
cannot be installed as binary package and must be built from source.
Install the Command Line Tools:
  xcode-select --install

Command Line Tools are already installed.
~> xcode-select --install
xcode-select: error: command line tools are already installed, use "Software Update" to install updates

We do not provide support for this pre-release version.


Read text from text dialog ends with blank screen

Hi, text context menu doesn't work when selecting text on webpages in Firefox. I  opens this screen and nothing is played.
TTS Util 4.0.0
Android 11
SM-A405FN

Hello Vladan,
Thank you for reporting this problem.  I have been able to reproduce it on
an Android 11 device running TTS Util version 4.0.0.  The same behaviour
occurs when using the 'Read text' share menu entry or the 'Read clipboard'
shortcut (on Android 7.1 to 9).
The problem appears to be related to issue #27, which causes an erroneous
text-to-speech initialization failure on Android 11.  Are you also having
that problem?  If so, it is fixed in TTS Util version 4.0.1, which should
be available from the Google Play or F-Droid store in the next few days.
There are some other cases where this behaviour can occur, e.g. if there is
a legitimate text-to-speech initialization failure.  I will add fixes for
those cases in TTS Util version 4.0.2.
On Sat, 13 Aug 2022 23:55:12 -0700
Vladan Kudláč @.***> wrote:

Hi, text context menu doesn't work when selecting text on webpages in Firefox. I  opens this screen and nothing is played.
TTS Util 4.0.0
Android 11
SM-A405FN

--
Reply to this email directly or view it on GitHub:
https://github.com/Danesprite/tts-util-app/issues/28
You are receiving this because you are subscribed to this thread.
Message ID: @.***>

Unfortunately, version 4.0.1 has still behavior.
App can see TTS engine and available languages and the app works.
I think the problem is related to too long strings.
When I paste the text into textbox, it's not read whole, it stops reading after few minutes.

Ah, you're right; I see it is still a problem in v4.0.1.  It seemed to be working for me earlier, but I had been testing with the wrong version.
This problem shouldn't occur if text is read/synthesised in TTS Util first, so that text-to-speech is initialized.  Of course, it will stop working again if your device kills the application in order to free memory.  In any case, the problem will be fixed in v4.0.2.
If you are having trouble with longer text not being read to completion, then perhaps the fault lies with the text-to-speech engine?  I've noticed this behaviour with some of them.  Which engine are you using with TTS Util?  Alternatively, try changing the application's Silent Utterances -> Line Endings setting to "Let the engine decide."

Hello @kudlav,
This issue is resolved in version 4.0.2, now available for download from the F-Droid store.  It is also available from the Google Play store, just not in Czechia.


Accessibility, more things

Everything done below is with the talkback screen reader. Which can be enabled in the "Accessibility" section of android settings

[x] The password box is completely skipped on the login screen. See the username box focuses in screenshot 1, the show/hide password focuses in screenshot 2 and the one time password (if enabled) focuses in screenshot 3.

[x] In screenshot 3 above the "one time password (if enabled box only reads as "edit box"

[x] In screenshots 4 and 5 the "forgot password flow is shown. I can access the back, email input box and send button, but have no idea what the text above the email box says as i cant access it

[ ] In screenshots 6 and 7 the "setup pin screen now has the delete button labeled, however, there is an extra button that just reads "button after it that still deletes. Notice the focus on what is read as "delete" is slightly wider than that of "button"

[ ] In screenshots 8 and 9 the "notifications" and "help" have been focused. Just after each is another object that just says "button".

!

[ ] In screenshot 10 an ad is open. After the "information" button is another object just labeled "button" that also opens the information page

[ ] In screenshot 11 the "copy" button is focused. Immediately after this copy button is another copy button.

[ ] In screenshot 12 the ad terms are in focus, however the only thing spoken is "read more" or "show less" depending on if the terms are collapsed or expanded.

[ ] In screenshot 13 the "show more" button is shown. This works as expected.

[ ] In the "wallet" tab and the "trades" the notifications button has that extra "button" after it again (see screenshot 8 above)

[ ] In screenshot 14 the arrow next to "all ads" reads as "button"

Edit: i found another one

[ ] in screenshots 15 and 16 a trade chat is displayed. Here the attach photo button (paperclip and send message button (up arrow in a circle) only read as "button"
[ ]

Outstanding job, thank you.
Will be done with the next release.

@sergdeus i found another one and added it to the bottom of the post

Thank you! Had some critical things to do. Next week will do this issue.

Hi!
About this issue
The password box is completely skipped on the login screen. See the username box focuses in screenshot 1, the show/hide password focuses in screenshot 2 and the one-time password (if enabled) focuses in screenshot 3.
I've tried accessibility mode with voice, and I should press this field and in this case, it works well.
How can I reproduce this issue?

@sergdeus Odd. It is not working for me. I attempted tapping the box directly ans swiping right with one finger and could not access it.
Edit: is the coding different from the username box other than hiding the input? Because for me the username box is fine. I am on the version ending in .37 if that helps

Yes, the coding is different.
I've added an additional focus request for a password field.
Please check does it help or not.
I can't check, because I can tap into this field with the old code as well.

@sergdeus nope. It would require a new app version to test

Yes, we will make a release tomorrow.

@sergdeus. Okay, i will test it then

So, the thing with the one time password box is now fixed. The password box is still skipped so i have unvhecked it. The forgot password text is read, but the focus is wrong (see screenshot. It has the focus on the entire app and not the element itself. It looks like this,

When it should look more like this

h the one time password box is now fixed. T

Ok., thank you.
I think I definitely should learn to test it by myself as well.
What is your Android version?

@sergdeus Android 11

I've figured out how to use this feature.
So, with the next update please check the Login, Sign up, and Reset password screens.
If they are ok I'll move further.

@sergdeus i will. Thanks for putting in the work. I know it can:t be easy dealing with this stuff

@sergdeus FANTASTIC! They work now. Login screen is fine, reset screen is fine. Only issue is that the "confirm password" box on the register screen only says "password" instead of "confirm password l" when using talkback. Otherwise perfect!

Great, thank you for testing.
In the next release, I'll finish all other issues.
For the new one - please open a separate thread.

@sergdeus will do


Chore: Update DevOps tooling from central repository [skip ci]

Update repository with content from upstream: os-climate/devops-toolkit

Don't update, as this breaks sigstore/gh-action-sigstore-python


Question

How to view the stats in the page?

Click on the provided link?

Isnt give me a link

Post a screenshot

Ok http://prntscr.com/6p9xws

http://prntscr.com/6p9yey

Is there a message in your logs?

http://prntscr.com/6pa0eq

I think this may be related to the client's set language

Fixed in 0402e3b15a51d891c3670adb2f87ae96640c68aa


SPFx w/SP2019 General Issues and aadHttpClientFactory Missing in 1.4.1

What type of issue is this?
Question
What SharePoint development model, framework, SDK or API is this about?
💥 SharePoint Framework
Target SharePoint environment
SharePoint Server 2019 (on-premise)
What browser(s) / client(s) have you tested

[ ] 💥 Internet Explorer
[ ] 💥 Microsoft Edge
[X] 💥 Google Chrome
[ ] 💥 FireFox
[ ] 💥 Safari
[ ] mobile (iOS/iPadOS)
[ ] mobile (Android)
[ ] not applicable
[ ] other (enter in the "Additional environment details" area below)

Additional environment details

browser version: 92.0.4515.159
SPFx version: 1.12.1
Node.js version: 10.20.1
NPM version: 6.14.4

Issue description
I'm trying to create a web part for SharePoint 2019 and SharePoint Online so I went through the template generator and it created a package but I noticed several issues and I'm not sure if the documentation is wrong on these or if the template generator is wrong or if I'm doing something wrong - the last issue is my main concern and what I need the most help with but I had to address the first 3 to get there:

The template generated dependencies on v1.4.0 of the SPFx components but my understanding is that v1.4.1 will work with SP2019. So I updated the various pieces to use 1.4.1 because I'm specifically looking for something that the docs state to be there (more on that in the last bullet).
If I delete the package-lock.json file and try to do an npm install (something that is necessary in many circumstances) there is a dependency added to @fluentui which breaks everything as it requires React v16 and so you get all kinds of typescript errors and dependency errors. This seems like a bug with something as the only way around this that I found was to create a new project, reinstall the dependencies, and then copy over the package-lock.json file to the old project.
The tslint.json in the root folder doesn't seem to be getting recognized and the only way I was able to fix it was to revert to the old structure with the file in the config folder. Not sure if I'm just doing something wrong here or if there's a bug with the generator or something else.
I was able to find workarounds for the previous issues which finally brought me to my real issue which is that I need to connect to a back-end service written using .NET 5 and secured using Azure AD; however, I immediately found that the AadHttpClientFactory class (and associated aadHttpClientFactory property on the WebPartContext object) are not present in the v1.4.1 package even though the docs state that it is (https://docs.microsoft.com/en-us/sharepoint/dev/spfx/use-aadhttpclient). You can clearly see that when looking at the package files:

I tried working around the fact that the API is missing by using ADAL.js but I've run into no end of issues trying to get that to work as the samples are all very dated and difficult to follow (and popups get blocked and you have to hack it to work in a non-SPA environment - all of which are bad).  Assuming the docs are just wrong does anyone have a sample that works with SP2019 and connects to a .NET 5 API secured with Azure AD (or even .NET 3 or something that actually works)?

I really appreciate any help that can be provided as I've been pulling my hair out on this for a while and it seems like it should be a simple thing but I feel like SP2019 stuff isn't really used much with SPFx or I'm just really doing something wicked wrong but there's not that much to creating a hello world web part which reproduces all of these issues.
Thanks again!
-Gary

Can you share your package.json file?

@patmill Here's the result of the npm list command:
+--<EMAIL_ADDRESS>+--<EMAIL_ADDRESS>| +--<EMAIL_ADDRESS>| | `--<EMAIL_ADDRESS>deduped
| +--<EMAIL_ADDRESS>deduped
| +--<EMAIL_ADDRESS>| | `--<EMAIL_ADDRESS>deduped
| +--<EMAIL_ADDRESS>| | `--<EMAIL_ADDRESS>deduped
| `--<EMAIL_ADDRESS>|   `--<EMAIL_ADDRESS>deduped
`--<EMAIL_ADDRESS>  +--<EMAIL_ADDRESS>  | +--<EMAIL_ADDRESS>deduped
  | `--<EMAIL_ADDRESS>  |   `--<EMAIL_ADDRESS>deduped
  +--<EMAIL_ADDRESS>  | `--<EMAIL_ADDRESS>deduped
  `--<EMAIL_ADDRESS>deduped

And here's the package.json
{
    "name": "test-spfx",
    "version": "0.0.1",
    "private": true,
    "main": "lib/index.js",
    "scripts": {
        "build": "gulp bundle",
        "clean": "gulp clean",
        "test": "gulp test",
        "serve": "gulp serve --nobrowser",
        "package": "gulp clean & gulp build & gulp bundle --ship & gulp package-solution --ship"
    },
    "dependencies": {
        "@microsoft/sp-core-library": "^1.4.1",
        "@microsoft/sp-lodash-subset": "^1.4.1",
        "@microsoft/sp-office-ui-fabric-core": "^1.4.1",
        "@microsoft/sp-webpart-base": "^1.4.1",
        "@pnp/spfx-controls-react": "^1.21.1",
        "adal-angular": "^1.0.18",
        "react": "15.6.2",
        "react-dom": "15.6.2"
    },
    "devDependencies": {
        "@types/react": "15.6.6",
        "@types/react-dom": "15.5.6",
        "@microsoft/sp-build-web": "~1.4.1",
        "@microsoft/sp-module-interfaces": "~1.4.1",
        "@microsoft/sp-webpart-workbench": "~1.4.1",
        "gulp": "~3.9.1",
        "ajv": "~5.2.2",
        "@types/webpack-env": "1.13.1"
    }
}

Thanks!

Thanks Gary  I can totally repro what you are seeing, and it hasn't changed in in maybe 2 years.  I'll try and sort out what is going on and post back here.

Thanks @patmill! Nice to know it's not just me :) - been doing mostly pure SPO stuff so was surprised when I hit the issues with SP2019 - I appreciate the help!

OK, so that's really awkward.  You are completely right.  OnPrem 2019 does not have a non-beta API for accessing the AADHttpClient.  Apparently you are the first person to notice this in years.  We will update our documentation, as changing the 2019 code is out of scope.  For the React 16 / @FluentUI, can you do an npm list to see where it is coming from?

Odd that nobody else has come across the fact that the API is missing - doesn't seem that much of an edge case. Do you think it would be possible to pull the source code for it and reverse engineer it into the src of my project or would that just open up a rabbit hole of craziness?
I've pasted the results of the npm list command below. Note again that this only happens if I delete the package-lock.json file, so it's easy to reproduce, but I should be able to do that without introducing breaking changes of this magnitude I would think.

OK, so you can't have webpart-base 1.12.1 in there (or anything referencing 1.12.1), it needs to stay at 1.4.1, can you change the reference to be absolute, rather than ^ based?

Obviously :)
So yeah, changing to the following (removing the ^) did the trick:
    "dependencies": {
        "@microsoft/sp-core-library": "1.4.1",
        "@microsoft/sp-lodash-subset": "1.4.1",
        "@microsoft/sp-office-ui-fabric-core": "1.4.1",
        "@microsoft/sp-webpart-base": "1.4.1",
        "@pnp/spfx-controls-react": "^1.21.1",
        "react": "15.6.2",
        "react-dom": "15.6.2"
    },

Unless I'm remembering incorrectly, when the template generates the package.json file it does so with the ^ in there - I wonder if that should be changed to remove it and have some documentation that states to use --save-exact when doing updates (like I had to go from 1.4 to 1.41 to try and find the AAD stuff). It's possible there's already something stating to use --save-exact and I just totally missed it but I've seen other projects in the pnp samples library, for example, that use "^1.4.1" instead of the exact reference so I know for certain I'm not the only one :)
Thanks again for all your help and if you have any thoughts on reverse engineering the AAD stuff into my project (or maybe as a shared package that can be leveraged by others) I'd appreciate it!

Yes - we stopped using the ^/~ references for all new SPO projects a while ago.  The next time we bump the on-prem generator we'll do the same.


EcmascriptEngine Duktape Pimpl

Bit of a big PR here, but most of the work is really just nudging some of EcmascriptEngine's private methods into a Pimpl struct, so much of this is renaming/re-indenting.
This PR touches on #62, #105, and #33, moving the Duktape implementation specifics entirely inside of blueprint_EcmascriptEngine.cpp. This should allow easier integration into user projects and better encapsulation of the Duktape dependency. While there, I changed the includes for duktape to use <> so that users can supply a custom include path if they want to integrate a particular Duktape build. We will continue to keep the existing Duktape source in the repo so that users who want the default can just target_include_directories(${TARGET} PRIVATE blueprint).
@JoshMarler let me know what you think, I know you were considering custom Duktape integrations for some time.
And @julianstorer if you have time, I'd love to confirm that this solves the problem you hit in #105. I have re-enabled Duktape's fast math error in this PR to restore parity with the distributed source, but I believe you'll now have the flexibility to work around it as you need.

@nick-thompson , all the code looks great. Question though, I could be wrong but I believe the cmake function juce_add_module(blueprint) will implicitly add blueprint as an include directory so I think you might get an include/header conflict with that approach?
Have you had a chance to test this? I realise duktape is currently in the module dir to comply with the JUCE module spec.
I could be wrong about this but there might need to be a slightly different approach for overriding the include <duktape/src-noline/duktape.h> header paths. Worth a test. I can pull this branch later and test including custom duk_config.h etc.

Question though, I could be wrong but I believe the cmake function juce_add_module(blueprint) will implicitly add blueprint as an include directory so I think you might get an include/header conflict with that approach?

Ah, interesting, I didn't see that but I can certainly imagine that would cause issues, and I only tested on MacOS.
One option could be to just move that duktape/ source up a level in the directory structure?

Yeah. I mean could it be moved up to externals with the other submodules? Then we could more easily version track duktape. The issue here being I think this breaks the JUCE module format? We should probably experiment with this a bit.

I mean could it be moved up to externals with the other submodules?

Yea but the issue with that is that we don't actually want duktape's git repository (IIRC), we want one of the tarball distributables.

The issue here being I think this breaks the JUCE module format? We should probably experiment with this a bit.

Right... hm. Maybe that's an ok break? I mean, the only ask for users is that they add an include path to their build. But... obviously the more we add up these "one little thing" changes the harder the overall "getting started" flow is.

Yea but the issue with that is that we don't actually want duktape's git repository (IIRC), we want one of the tarball distributables.

What's the reason for this out of interest? Can we not just set the submodule to one of these:
https://github.com/svaarala/duktape/tree/v2-maintenance
https://github.com/svaarala/duktape/tree/v2.6-maintenance

Right... hm. Maybe that's an ok break? I mean, the only ask for users is that they add an include path to their build. But... obviously the more we add up these "one little thing" changes the harder the overall "getting started" flow is.

Yeah, lemme check the shape of JUCE modules under cmake again. I'm not sure how much flexibility is allowed for dependency include paths etc.

See here: https://github.com/svaarala/duktape/tree/v2-maintenance#getting-started-end-user. Duktape just doesn't include the prebuilt source files in its git repository. If we submoduled, there would be an additional step for end-users to build the source distributable after cloning blueprint

See here: https://github.com/svaarala/duktape/tree/v2-maintenance#getting-started-end-user. Duktape just doesn't include the prebuilt source files in its git repository. If we submoduled, there would be an additional step for end-users to build the source distributable after cloning blueprint

Ahhhh right OK. I hadn't appreciated this. Think we probably need to give this all a bit of a test. Configure our own duktape setup and see how easy/painful it is to do a custom include in user project.

I think I have a better way to handle all of the warnings, configurations for Duktape, and so on...
@nick-thompson If you don't mind holding off on this PR, I'm in the middle of a large clean-up of the module itself to make it more C++-like and more Unity build friendly as a whole.

@jrlanglois interesting, can you share what you've got in mind?

Well, it's one of those things that's elaborate and I didn't break it down entirely. Might be easier to push up a branch to explain/get a biew on what I'm concocting.

@nick-thompson, be cool to see this go in if we're tackling duktape as a separate issue. All the code looks tasty to me. Call me mad but I've been looking over embedding V8 docs ..... Minor concerns Duktape might never fully cut it with the GC overhead on our embedded targets and I'd be really really interested to see what performance with V8 or other engines is like.

@JoshMarler thanks! Yea, I'm happy to merge this and follow up separately with nudges for the includes + trying to optimize the config options for integrating a custom duktape build.
@jrlanglois does that sound ok to you?

+1 !

🚀 🚀


Use daggy instead of union.js

Codecov Report

Merging #38 into dev will decrease coverage by -0.36%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev      #38      +/-   ##
==========================================
- Coverage   98.94%   98.59%   -0.36%
==========================================
  Files           7        6       -1
  Lines         189      142      -47
==========================================
- Hits          187      140      -47
  Misses          2        2

Continue to review full report at Codecov.

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


IE8 outside of an iFrame never connects to Karma Server

Expected behaviour
Karma can connect to a Sauce Labs session running on IE8, Windows 7 and execute a simple test suite.
Actual behaviour
Karma will connect, then timeout waiting for messages:
12 09 2016 01:36:38.416:INFO [launcher.sauce]: internet explorer 8 session at https://saucelabs.com/tests/0372bb42efb2493895bcc86b225605c1
12 09 2016 01:36:47.889:INFO [IE 8.0.0 (Windows 7 0.0.0)]: Connected on socket /#oG2ATJ5URrm38A7VAAAA with id 62797665
12 09 2016 01:36:57.894:WARN [IE 8.0.0 (Windows 7 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
IE 8.0.0 (Windows 7 0.0.0) ERROR
  Disconnected, because no message in 10000 ms.
IE 8.0.0 (Windows 7 0.0.0) ERROR
  Disconnected, because no message in 10000 ms.
IE 8.0.0 (Windows 7 0.0.0): Executed 0 of 0 DISCONNECTED (10.006 secs / 0 secs)

Environment Details
Sauce Labs' infrastructure

Karma version (output of karma --version):
Karma version: 1.3.0.

Also tested, 1.2.0, 1.1.0, 13.22.0

Relevant part of your karma.config.js file

// Browsers to run on Sauce Labs
  var customLaunchers = {
    'SL_InternetExplorer': {
      base: 'SauceLabs',
      browserName: 'internet explorer',
      version: '8'
    }
  };

<snip>

config.set({
  client: {
    useIframe: false
  },
  customLaunchers: customLaunchers,
  browsers: Object.keys(customLaunchers),
  singleRun: true

Steps to reproduce the behaviour

Configure a Karma suite according to these instructions for Karma-sauce-launcher.
Use the above customLaunchers to kick off a Karma session.
Sadness and Woe as your tests time out.

Additional Information
I work for Sauce Labs and I've spent a couple of hours digging into this.  By starting Karma without being in Single Run mode, I was able to find that the non-iFrame version was causing JS errors in context.js.:
Webpage error details

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; WOW64; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET4.0C; .NET4.0E)
Timestamp: Mon, 12 Sep 2016 08:52:15 UTC

Message: JScript object expected
Line: 239
Char: 5
Code: 0
URI: http://localhost:9876/context.js

That method appears to have to do with proxying Karma methods to the main window?  From this error description: https://msdn.microsoft.com/en-us/library/yt9ydd55(v=vs.94).aspx I did some digging (and have now lost the relevant link) and it appears that maybe Window isn't a JS object for IE8, hence the error.  And that exhausted my JS capabilities.

Is there a specific reason you are using iframe: false?


Internal: Refactor away show_commit_info from LogPanel

It is a lot more convenient to just set on_highlight.
When you need a different on_highligh you just overwrite the current one. Either
by a new call or as a kwargs to show_log_panel
For #956 to go easier

@randy3k @asfaltboy

In this way, we cannot disable show_commit_info by passing on_highlight=None.

You would need to give it a callable like on_highlight=lambda c: None

Maybe we could change line 426 to detect if kwargs https://github.com/divmain/GitSavvy/blob/c67bf8cc6ad981d5bd7e8f144f2dec4752f62177/core/ui_mixins/quick_panel.py#L360
has on_highlight

Your are probably right, but a more straightforward solution is to check kwargs.
if "highlight" is not in kwargs:
    # default highlight
else:
    if kwargs["highlight"]:
        # use this
    else:
        # disable highlight

What about
def show_log_panel(entries, on_done, **kwargs):
    """
    Display log entries in quick panel with pagination, and execute on_done(commit)
    when item is selected. `entries` can be either a list or a generator of LogEnty.

    """
    _kwargs = {}
    for option in ['selected_index', 'limit']:
        if option in kwargs:
            _kwargs[option] = kwargs[option]

    for option in ['on_highlight']:
        if option in kwargs:
            _kwargs[option] = kwargs[option]
            if not callable(kwargs[option]):
                _kwargs[option] = lambda commit: None

    lp = LogPanel(entries, on_done, **_kwargs)
    lp.show()
    return lp

it also works (but the second for loop has only one item so it is not necessary)

I will commit is like
def show_log_panel(entries, on_done, **kwargs):
    """
    Display log entries in quick panel with pagination, and execute on_done(commit)
    when item is selected. `entries` can be either a list or a generator of LogEnty.

    """
    _kwargs = {}
    for option in ['selected_index', 'limit']:
        if option in kwargs:
            _kwargs[option] = kwargs[option]

    if not callable(kwargs['on_highlight']):
        _kwargs[option] = lambda commit: None

    lp = LogPanel(entries, on_done, **_kwargs)
    lp.show()
    return lp


Reunion 0.8 preview templates generate reference error for Package project

Describe the bug
Visual Studio template for Reunion 0.8 preview seems to be incorrect in some way.
Getting Project must have a reference to an application. Please add a reference under the 'Applications' node in the Solution Explorer.	App2 (Package)	C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\DesktopBridge\Microsoft.DesktopBridge.targets	742 while the projects are clearly linked together.
Steps to reproduce the bug

Steps to reproduce the behavior:

Generate a new Project Reunion project based on the 0.8 preview template for C++/WinRT
Build solution

Expected behavior
Everything compiles properly.
Screenshots

Version Info
0.8-Preview
NuGet package version:
0.8.0-preview

Hey @aeloros, can you look into this?

yup
From: Andrew Leader @.>
Sent: Thursday, June 3, 2021 12:17 PM
To: microsoft/ProjectReunion @.>
Cc: Scott Darnell @.>; Mention @.>
Subject: Re: [microsoft/ProjectReunion] Reunion 0.8 preview templates generate reference error for Package project (#905)
Hey @aeloroshttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Faeloros&data=04|01|scodar%40microsoft.com|1f68ef07de5849ad9f0b08d926c43697|72f988bf86f141af91ab2d7cd011db47|1|0|637583446448107416|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=FZC1HepOAAvwwO88iD%2F2WPHNyhml7Qouql6JZi%2FIG2k%3D&reserved=0, can you look into this?

You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fmicrosoft%2FProjectReunion%2Fissues%2F905%23issuecomment-854114658&data=04|01|scodar%40microsoft.com|1f68ef07de5849ad9f0b08d926c43697|72f988bf86f141af91ab2d7cd011db47|1|0|637583446448107416|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=NC7OHyGwYR0dYcJUpSs6wn9WkQmCKAb1pSui7imh7g0%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAE4I5UQ2DZIEEQHCH5CQDXDTQ7IMDANCNFSM46A6XZYQ&data=04|01|scodar%40microsoft.com|1f68ef07de5849ad9f0b08d926c43697|72f988bf86f141af91ab2d7cd011db47|1|0|637583446448117371|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=%2BtQuXTezURGqZjMIJxAv8i%2B5QLc8WSm9QUd0GxXf%2FHI%3D&reserved=0.

@pjmlp I'm unable to reproduce the problem you're seeing.  Here is a snip of my screen showing the successful build and the version of VS I'm using.

This is the project I chose for testing purposes.

In your repro what project are you using, and version of VS?
Thanks
--Scott

@aeloros
Just reproduced it, however the error is not on the template as I thought.
Rather when the project is fully created, we get the Visual Studio as shown below.

When closing the package overview, we get asked if we want to save, which is kind of odd as nothing was changed.

However apparently it was, and not saving will produce a corrupt packing project, as EntryPointProjectUniqueName won't be present on the project file.
I am also attaching the build log and generatd project.
I guess the bug would be not to have such change take place as many developers would do what I did and press "Don't Save" without realising the consequences.
App5.zip
Output.log

If you manually hit save later does it fix it, or are the changes lost somehow after these actions?

Nevermind, I was able to repro the problem now.  Hitting 'Save All' afterward does not fix the build error. :(

Closing this issue as it isn't Windows App SDK related and now has internal tracking.


How do I use this in the browser?

Hi. How do I use this in the browser? I'm looking for a single js file I can add to a simple small html page using a <script> tag. I haven't installed requirejs or any other libraries yet, I just have a hard coded CSV string that I'd like as a json. I'm trying to keep things as simple as possible, without using webpack or bundling/minification. What I'm looking for:
<html>
    <head>
        <title>CSV to json example</title>
    </head>
    <body>
        <!-- csvtojson script here -->
        <script src="csvtojson.js"></script>
        <!-- my script -->
        <script src="script.js"></script>
    </body>
</html>

Thanks for any help!

Please check document: https://github.com/Keyang/node-csvtojson#browser-usage

Close due to no activity

@Keyang there is no csvtojson.min.js any more. Is something missed in webpack.config.js?


Fix for: ogg importer dialog rounds loop offset values to 3 digits, introducing a sampling imprecision that can lead to an audible pop on loop.

Fixes #87427 for ogg and mp3 files, by introducing a toggle that allows the user to enter their loop offsets in either seconds or in samples. AudioStream classes are mostly left unchanged, except that I had to add 2 methods (get_sample_rate and sample_to_seconds) to make this work. Wav stuff was left unchanged.
I also took the liberty to make the importer dialog a little nicer to use, by making the loop offset field a little larger, and making loop-related stuff invisible when looping is disabled.
Disclaimer: I'm a total git newbie, so hopefully I did everything right.

Production edit: This closes https://github.com/godotengine/godot-proposals/issues/84.

Ok I know I am not an audio contributor but I do review some of the asset pipeline changes and I want to understand this change a bit better.
How does this relate to bpm support such as #63265? My fear is making the loop offset be in samples will make it more difficult to calculate the beat syncing which is in units of BPM (seconds)
I'm ok with making this use samples internally to be consistent with wav, but asking the user to type samples is not good UX (and if they use seconds this bug is not fixed)
If it shows seconds, we should not be setting a step on the SpinBox (or EditorSpinSlider), or if we do use a step, it should be within 1/sample rate but this looks ugly too)... it should be possible to pick a value for step size that guarantees no precision loss at common sample rates (for example, 1/100000)
i do not like a boolean that affects the meaning of another value. I would rather deprecate the existing seconds loop? Instead, add a new loop_offset_samples and have an interactive UI for playing or inputing a loop point, with options for samples, seconds or beats (in BPM units)
Generally speaking, I would hope for some way to make this more interactive.
Why is a bad seek offset causing a pop? Being off by one sample shouldn't cause a pop. It must be off by far more than 1 sample... if the issue is that the spin slider is set to 0.001 that is 1/1000 which is 48 samples, that is far too coarse to accurately pinpoint a loop point unless we are able to round to beats.
If so, we could start with a simpler change and use 0.00001 for the spin slider or SpinBox (why is it not using EditorSpinSlider?) and see if that solves the popping issue. Then this change boils down to UX improvement (selecting beats)
The presence of pops when we get this wrong is not ideal. When I seek youtube videos, I don't hear pops. And when I seek haphazardly in DJ software it usually rounds to the nearest beat which perhaps avoids pops too. I wonder if it might be good to make godot more fault tolerant to bad seek points.

Hey, thank you for your reply.
This PR should not interfere with BPM support. It basically only expands the function of the importer's loop offset field, while the AudioStream classes are mostly left unchanged (beyond the two added getter methods). In my view, both beat-based loop points and sample/second-based loop points are desirable features; the former being more useful for songs while the latter is more useful for things like background noise that don't follow a clean beat structure.
And yes, the issue is indeed that the 0.001 step is too coarse. My initial thought was thus of course to simply increase the step value of the seconds field like you suggest, but this introduced some really weird rounding issues of an entirely different kind that utterly confounded me (please see issue page #87427, I get the impression you may know more about UI stuff than I do, and I'd really like to address it). I discussed with @bs-mwoerner, and we felt that adding a toggle for sample-based inputs is a good compromise for now.
As a composer I actually generally prefer using samples over seconds for situations where using beats is not possible or desirable, but that's a personal preference thing.
I like your idea of a more interactive unit selection scheme, though maybe this is beyond the scope of this PR? Could this be a good idea for a feature proposal perhaps?

First of all: We can and should discuss what the best way is to achieve this, but I am indeed a strong supporter of the idea itself. "Strong supporter" as in my current workflow requires me to edit media in external software to accomodate a limitation of the Godot Editor UI (i.e. a limitation that's not in the Engine itself). Godot is a fantastic piece of software and this is by no means a showstopper, but the workaround feels a bit irksome after some time and there's no doubt Godot could be even better if that workaround would not be necessary.
So, before going into the discussion of how, I'd like to address the why in more detail.
Why do you care as much about where exactly your loop point is?
I see two distinct use cases:

Working with recorded audio or audio provided by a third-party where you guesstimate a suitable loop point by fiddling around with the settings until it sounds right. For this use case, seconds is a good unit and millisecond resolution is probably adequate.
Working with generated audio (digital music, synthesized sound effects) that has been produced to seamlessly loop between two specific positions in the stream. For this use case, seconds is a bit of an awkward unit, because integral stream positions will generally not convert to clean time fractions, and millisecond resolution is not nearly enough to identify a specific point in the stream (when thought of as a sequence of audio samples).

Note that case 1 can transition to case 2 when you want to make your loop cleaner by externally baking a little fade in. Then you do require the loop to line up with the fade you created.
A millisecond covers about 40-50 samples. Can you even hear that difference?
Very much. I recently fixed a bug in the WAV stream (not yet merged I think) where users reported cracks in a loop that should be seamless and the issue was that when subsampling at the end of the loop, the stream interpolated with the first sample after the loop instead of with the sample at the loop start. You might argue that this was an error of less than one sample and it still produced an audible defect in the playback.
The technical reason is that while you can't identify individual samples when listening, discontinuities in the waveform caused by a sudden change in the value of the curve produce an impulse in the signal, which translates to a very short but high energy noise (i.e. a signal containing all frequencies) and can be clearly heard even at such short durations. That's why audio "noise" can be generated by filling a stream with random values. This will produce a series of impulses at varying intensities.
Bonus fact: Digital audio obviously uses numbers to represent the wave form, so it's not actually a curve, it's a series of small stairs, all with a tiny vertical flank to the next sample. This actually produces noise similar to the imperfect loop crack and the intensity of that noise depends on the quantization of the signal. At 16 bits that quantization noise is at about -96 dB, so not really a factor, but for a 1-bit quantized waveform (which surprisingly is enough to understand speech), this quantization noise is very noticeable.
But does looping need to be perfect in the first place?
The thing is that it can be perfect from the source media's point of view and it can be perfect from the engine's point of view. It currently is imperfect purely because the editor does not let you select the correct loop point, which can feel a bit frustrating.
Then how about we add a bit of fading around the loop points to hide the pop? Isn't that what video players do when you seek in the stream?
Please don't do this generally. If the stream has been engineered so that the ends match perfectly, lining them up wrongly and then adding a big blur on top to hide that fact is not an ideal solution. If you have arbitrary audio, on the other hand, such as when seeking in a video stream or looping audio without a dedicated loop point, then there is no "right" way to align the parts and that "blur" is indeed very welcome.
So what about the bpm settings? Can't you use those instead?
In theory, this would work for music that includes no tempo changes and has no lead in, which, to be honest, is probably true for the majority of game music. In practice, however, from my own experience and from looking at the code, I'm not entirely sure what the bpm setting does. It seems to me that it's not related to looping (that's still determined entirely by the "Offset" setting) but rather crops the stream at the end. Unless I misunderstand this feature (which is very possible), this is meant for cases in which you want to loop or stop before the actual end of the file (specified in beats/bpm). It has no effect (positive or negative) on your ability to set the loop point (still specified in milliseconds).
Isn't "samples" a very weird unit? Can't we just allow more digits in the "seconds" input?
From a technical point of view, "samples" is the "native" unit: A stream is a list of samples and each sample in the stream has an index, just like every byte in a file has an index. You can convert this to a time unit by doing some floating point math, but knowing the percise index and having to convert it to some weird floating point number is about as fulfilling as trying to advance 27 bytes in a file with a MoveFilePointer(double distance) function that takes the distance to move as "a fraction of the file size". Still, allowing more digits would indeed solve the problem. If you have a 44.1 kHz file and a loop point at position 40,000, you'd have to be able to enter 0.90703 into the field and this should then produce the correct value when the stream later converts it back to an integral position (39,999 is 0.90701 and 40,001 is 0.90705).
Since this doesn't seem to be a major problem at the moment, what's the workaround?
The workaround is to bake parts of the loop into the source audio by adding samples from the start of the loop to the end of the file until the stars align and the loop point happens to fall on a whole or tenth or so second. Then import that file into Godot by specifying a somewhat clean "seconds" loop point. It's a workable solution as long as you use a tempo where a beat and a millisecond align at some point, but it's still a bit of a hassle considering that the only reason for doing this is just a UI issue.

Ok, in response to the above discussion, I've increased the step value of the loop offset field to 0.00001, which if I understand correctly is enough precision to cover all commonly-used sample rates.
Of course, the aforementioned rounding error on reimport is still there. I suspect it may actually be a deeper-lying problem with how the engine imports floats from resources, but I need to investigate this further - if so, I think I'll make that a separate issue / PR, since it'd be a wider-reaching change.
Still, exposing the issue is probably better than hiding it behind a 0.001 step value. This is a change that we will need to make eventually anyway in order to fully resolve this issue for seconds-based offsets, so I figure we might as well make it now.

Floating points have a 23 bit mantissa
2^24/48000/60 is around 5.8, just under 6 minutes.
That means, even in the best case, you will be unable to accurately represent a sample in a track 6 minutes long if we use floating point here.
combined with @bs-mwoerner's comment, this might be good justification for using samples, since an integer has at least 32 bits of precision.
so the real issue I have with this PR is that we are shoehorning two types of data into the float loop_offset which has insufficient precision.
I would suggest changing this to make loop_offset_samples as an int32 or int64 and have some logic to decide which to use...
one suggestion I have is to perhaps deprecate loop_offset (use it if nonzero but keep loop_offset_samples in sync if possible). But that would need wider consensus

There is a world where we turn this into a huge refactoring that unifies WAV, OGG, and MP3 import but if we prefer to keep this a minimal as possible for now, I think we don't need to change anything with the internal data structure and just need to add a drop down that lets you select between "seconds", "samples" and "beats". Changing this would only change the conversion that is used for the Parse/ToString of the input field, while internally it's always stored as a seconds value (so as to maintain full compatibility). This would still require the change that makes the streams expose their sample rate, but other than that, this wouldn't overthrow a whole lot. We wouldn't even need a persistent loop_use_samples or any other new attributes.
For that to work, though, we need to make sure that the precision of the value actually survives into the audio stream. I think it does, but you get a truncated version when you open the dialog again because the dialog takes its settings not from the resource itself but from the .import file and there float values are serialized via snprintf(buf, 256, "%lg", p_num), which outputs a limited number of digits. If we want to work around that, we may have to move to storing sample counts as an int64 after all, although that would only affect the .import file, can be made backwards compatible, and requires no API changes on the actual resources.

Well, the simplest improvement would be to use double consistently. As the audio containers work with double anyway. Though some importers still use float.
I reckon/hope saving a double outputs a higher precision?!

I reckon/hope saving a double serializes to a higher precision?!

I haven't actually tested this, but from looking at the code, the configuration values for resource importers get stored in a Variant, which internally always uses double for floating point values, so the precision probably isn't lost before the actual conversion to a string during writing out the file very deep down in
https://github.com/godotengine/godot/blob/e0eccaeb60b40fd7aedd272e5c9dfff123e42c1a/core/string/ustring.cpp#L1678
Integers of any kind, on the other hand, get serialized in
https://github.com/godotengine/godot/blob/e0eccaeb60b40fd7aedd272e5c9dfff123e42c1a/core/string/ustring.cpp#L1589
and this seems to keep the full 63 bits + sign precision, which would be enough to precisely address samples in audio files with a play time of less than 6 million years.
Hm, I wonder if .tres files take the same path.

@bs-mwoerner: I made a PR to iron out double usage (and remove rounding enforced by the SpinBox): https://github.com/godotengine/godot/pull/87860. Let's see if that's enough (for now)

I've tested @MJacred's PR (see here). Unfortunately, it doesn't resolve this issue.
I'm going to test @bs-mwoerner's idea and see if I can confirm that the problem occurs in ustring.cpp.

Ok, I've confirmed that double values correctly enter ustring::num_scientific, but have become truncated in the buf variable that then gets returned. So indeed, the problem lies pretty deep in Godot's internals. There's some compiler-dependent code here, too, which makes me a little nervous.
So, I see two ways forward here:

Somehow alter the behaviour of num_scientific and/or its dependencies to serialize doubles with more precision.
Go with an integer-based .import parameter like previously suggested.

I think I prefer the latter (since there's less potential for breaking stuff), but some discussion is definitely needed here. In either case, opening a new issue for the double serialization problem is probably a good idea.

I've now had a look at option number 2, whether we could actually use an INT option to bypass the limited precision in the FLOAT serialization. We would still need to expose the sampling rate from the audio streams, but at least we wouldn't have to touch String or the serialization or similar fundamental classes. I've created yet another pull request #87882 that mostly builds upon this one (although I added a third option for entering "Beats"). I've named you as a co-author, I hope that's okay. I'm hoping that this addresses both the input issue and the survive-through-reimport issue and I'd be glad about anyone willing to test it.


Add a new API version (2022-01-01-preview) for microsoft.app RP

MSFT employees can try out our new experience at OpenAPI Hub  - one location for using our validation tools and finding your workflow.

Changelog
Add a changelog entry for this PR by answering the following questions:

What's the purpose of the update?

[X] new service onboarding
[X] new API version
[ ] update existing version for new feature
[ ] update existing version to fix swagger quality issue in s360
[ ] Other, please clarify

When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month.
January 2022
When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month.
January 2022
If updating an existing version, please select the specific langauge SDKs and CLIs that must be refreshed after the swagger is published.

[ ] SDK of .NET (need service team to ensure code readiness)
[ ] SDK of Python
[ ] SDK of Java
[ ] SDK of Js
[ ] SDK of Go
[ ] PowerShell
[ ] CLI
[ ] Terraform
[ ] No refresh required for updates in this PR

Contribution checklist:

[X] I commit to follow the Breaking Change Policy of "no breaking changes"
[X] I have reviewed the documentation for the workflow.
[X] Validation tools were run on swagger spec(s) and errors have all been fixed in this PR. How to fix?

If any further question about AME onboarding or validation tools, please view the FAQ.
ARM API Review Checklist

Applicability: :warning:
If your changes encompass only the following scenarios, you should SKIP this section, as these scenarios do not require ARM review.

Change to data plane APIs
Adding new properties
All removals

Otherwise your PR may be subject to ARM review requirements. Complete the following:

[X] Check this box if any of the following apply to the PR so that label "WaitForARMFeedback" will be added automatically to begin ARM API Review. Failure to comply may result in delays to the manifest.

Adding a new service
Adding new API(s)
Adding a new API version
-[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits.

[X] Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.

[ ] If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist
If any of the following scenarios apply to the PR, request approval from the Breaking Change Review Board as defined in the Breaking Change Policy.

[ ] Removing API(s) in a stable version
[ ] Removing properties in a stable version
[ ] Removing API version(s) in a stable version
[ ] Updating API in a stable or public preview version with Breaking Change Validation errors
[ ] Updating API(s) in public preview over 1 year (refer to Retirement of Previews)

Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.
Please follow the link to find more details on PR review process.

@msyyc - the API Readiness check fails because we haven't onboarded the RP yet. In order to onboard we need to upload an ARM Manifest and the prereq to the ARM manifest is the swagger PR signoff.

@msyyc - this is the same set of changes that were already approved in the private repo: https://github.com/Azure/azure-rest-api-specs-pr/pull/5629

Created a different PR to a separate branch

https://github.com/Azure/azure-rest-api-specs/pull/17135


Fix issue where header search would fail on initial empty values

Regression from #9768
Fixes #9918 (see for steps to reproduce)

@Lovelyfin00  FYI - the changes on #9768 introduced a bug, small fix but maybe just have a look so you are aware of what needed to change.
Sorry I missed this on our review.

Handled by #9923

@Lovelyfin00 FYI - the changes on #9768 introduced a bug, small fix but maybe just have a look so you are aware of what needed to change.
Sorry I missed this on our review.

Looking through the change
Thanks👍


fix: drawer item styling

sets icon and text color according to material design guideline.
removes unused bottom navigation selectable color drawable.

fixes #282

@iamareebjamal please review

@mejariamol Shouldn't they both be of same color?

according to guidelines icon shall have 54% alpha and 87% for text.


Update info.md

Updated the info.md since i noticed that it redirected, becuse the master branch does not exists anymore.

Thank you!


Sync and update golangci-lint configurations

Readiness checklist

[ ] I added tests for new functionality or bugfixes.
[x] I ran task all, and it passed.
[ ] I added/updated comments for both exported and unexported top-level declarations (functions, types, etc).
[ ] I checked comments rendering with task godocs.
[x] I ensured that the title is good enough for the changelog.
[x] (for maintainers only) I set Reviewers (@FerretDB/core), Assignee, Labels, Project and project's Sprint fields.
[x] I marked all done items in this checklist.

There is a bug, will fix after release


Add roki-glfw & sample

Regarding issue #6, Added a function to use GLFW as an alternative to GLUT. I tried to make the functionality the same as GLUT.
The sample is example/glfw/glfw_solid_test.c. This is just an imitation of example/glut/glut_solid_test.c. However, this sample lacks more examples than the original GLUT one.
Please confirm.

I thought it would be better to have common parameters (including a pointer to the default camera) and share them for GLUT and GLFW. Please check src/rkgl_camera.c in the latest version of the main branch. I think you can modify your roki_glfw.c by referring the new roki_glut.c.
Regarding glfw_solid_test.c, rkglBox() is available. See example/shape/box_test.c. Also, why don't you test your rkgl*FuncGLFW() in the program?

roki_glfw was also updated according to the update of rkgl_camera.
I also used the rkgl_input global variable and updated the callback function.

I merged this into the main branch with some modifications.
A major modification is to rename rkglWindowCreateGLFW() to rkglWindowOpenGLFW(), and add another function rkglWindowCreateAndOpenGLFW() in order to avoid confusions.
If you agree with this, I'll close this PR.

@zhidao
Thanks your merging.
I agree with the modification that rename rkglWindowOpenGLFW() and add rkglWindowCreateAndOpenGLFW().
It's OK to close this PR.


[CONTENT] Tutorials - Building a minting and marketplace app - Flow

Alongside #145, it would be cool to have a tutorial for @jochasinga 's pet shop tutorial on Flow (Flowwow). This is a placeholder issue for that tutorial, covering the minting service part and the marketplace part of Flowwow for devs new to NFTs and Flow.

This should be closed @dchoi27


Timepicker issue

Fixes issue #1267
We fixed the issue with the start time and end time buttons not triggering the timepicker dropdown attached to the input field. The issue was the timepicker method in the timepicker object didn't expect "show" as argument. Instead, it expected "open". Moreover, we prevent the event handler from propagating up because it would close the dropdown as in the implementation of the timepicker closes the dropdown if it's not trigged from the input field directly.

To add on to Ali's comment, we also implemented an if else statement into the function bound to the button because the button would open the time picker scroll but clicking it again would not close the scroll box, so we used the time pickers 'closed' method in order to check if the scroll box was open or not, and close and open it appropriately.

Corrected things mentioned in comments

There are some awful merge conflicts. Going to close this and reimplement the changes in development.


PyCompileAndRuntime does not work.

I tried to run the mnist example by using PyCompileAndRuntime, but it didn't work:
$ cd docs/mnist_example
$ python mnist-runPyCompileAndRuntime.py
Traceback (most recent call last):
  File "mnist-runPyCompileAndRuntime.py", line 11, in <module>
    session = OMCompileExecutionSession(inputFileName,sharedLibPath,"-O3")
RuntimeError: Cannot open library: './mnist.so'.

According to PyCompileAndRuntime definition here
PyOMCompileExecutionSession::PyOMCompileExecutionSession(
    std::string inputFileName, std::string sharedLibPath, std::string flags,
    bool defaultEntryPoint)
    : onnx_mlir::PyExecutionSessionBase(sharedLibPath, defaultEntryPoint) {
}

It constructs a PyExectionSessionBase with a .so file (sharedLibPath) that does not exist at the construction time.

Something changed, and it's not working anymore. Easy fix, working on it.


Redirect to an additional form page after Authentication.

Hi, I would like to have a page, which asks users to input additional profile info, after authentication.
Facebook user info have first name and last name, but Twitter user info don't, but I want to save the same info to database.
The page flow that I am thinking is something like this:

Login page(click 'Login with Twitter account') ->
Twitter authentication page(allow using the profile info) ->
Additional info input page(input additional info like first and last name, then click send) ->
LoginRedirect page.

Would anyone please help me out to make it possible?

just a thought..
you could have a check after login that if certain information is not there users are redirected to a update profile page. so its a general redirection(your loginredirect url will same for all) and when user logs in with social profiles it will also work.
meanwhile, this is not a bug :)

you could have a check after login that if certain information is not there users are redirected to a update profile page. so its a general redirection(your loginredirect url will same for all) and when user logs in with social profiles it will also work.

This.


[WIP] Dependencies update

Dependencies update

Codecov Report

Merging #49 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #49   +/-   ##
=======================================
  Coverage   42.95%   42.95%
=======================================
  Files          48       48
  Lines         433      433
  Branches       46       46
=======================================
  Hits          186      186
  Misses        214      214
  Partials       33       33

Continue to review full report at Codecov.

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


support targeting children of unary/binary operators

Cursorless has a state scope but no expr scope. Expressions are too dense and nested to be useful targets. In particular, it’s hard to target them precisely.
One exception is the children of unary/binary operators. We could support left/right scopes, where the domain is the operator and the content range is the subexpression. This would enable things like swapping the order of conjuncts.

Maybe we could do the following:
     (a + b) + c
//!1  ^
//!2      ^
//!3  *****
//!4 ^^^^^^^
//!5           ^
//!6 ***********

Then you could say (assuming scope is called "operand", first + has gray hat, and second + has blue hat):

"first operand plus" to get a,
"last operand plus" to get b,
"first operand blue plus" to get (a + b), and
"last operand blue plus" to get c.

Then we don't need all the specialness around "left" / "right" / domain being contained by range, etc.
I don't feel strongly tho if you like your syntax better

We might want to think a bit more carefully bout associative operators. Should prob be able to say things like "fourth operand", etc, even if parse tree uses a car-cdr style hierarchy

Note that in my proposal, "next operand plus" / "previous operand plus" should work as well

My gut reaction is as follows. Happy to discuss more.

We might want to think a bit more carefully bout associative operators. Should prob be able to say things like "fourth operand", etc, even if parse tree uses a car-cdr style hierarchy

I think this gets complicated.
What should "third operand plus" be in each of these scenarios, assuming the default hat is always over the leftmost +?
a + b + c
(a + b) + c
a + (b + c)
a + b - c
a + b * c
I'm guessing it is only well-defined in (1), despite the semantic equivalence of (1) to (2) and (3) and the syntactic similarity of (1) and (4) and (5).
Also, how do you easily explain to users that the first plus in a + b + c has a third operator, but the first dash in a - b - c doesn't?
I see the appeal of it, but I think it ends up mixing up syntactic and semantic notions in a way that may end up confusing.

even if parse tree uses a car-cdr style hierarchy

I would be astonished to learn that parse trees for binary operators did anything but.
I think users will very quickly learn that a + b + c is implicitly a + (b + c), and from there everything else is pretty intuitive.
If you buy this, then there are only ever first operand and second/last operand, which are clean and clear from the grammatical perspective but also a lot of syllables vs left/right.

"chuck first operand blue plus" to leave just c

This is really nice. I can see using this a lot.

Define this as a set of modifiers
How to define "left" as a modifier, given the above iteration / scope ranges
iterationRange = containing(iterationScopeHandler, input)
candidates = scopeHandler.generateScopes(editor, input.end, "forward", {
  // might be able to drop next line
  includeDescendantScopes: true,
  distalPosition: iterationRange.end,
})

return largest candidate

note: also want to support just getting the containing iteration scope itself


mse needs updating (to friedman_mse?)

https://github.com/scikit-optimize/scikit-optimize/blob/a2369ddbc332d16d8ff173b12404b03fea472492/skopt/learning/forest.py#L197
It seems this function no longer works with the latest sklearn because mse no longer exists

I'm having the same issue. I believe the correct criterion value would now be "squared_error". I'd love it if you guys would make this change - thanks!


Is there a way to rename or choose AWS resource names?

Please describe which feature you have a question about?
to rename or choose S3 bucket, Cloudfront, Lambda@Edge names
Provide additional details
I'm using Amplify to host Next.js apps. But deployed apps always have non-human-readable resource names on S3 Bucket, Cloudfront, Lambda@Edge.
Is there a way to rename or choose AWS resource names?
What AWS Services are you utilizing?
Amplify, S3, Cloudfront, Lambda@Edge, etc.
Provide additional details e.g. code snippets

Unfortunately, there is no way to provide descriptive names. Sorry for the inconvenience.


Datepicker calendar breaks down in firefox

Bug, feature request, or proposal:
Bug
What is the expected behavior?
Datepicker calendar should works in firefox.
What is the current behavior?
Datepicker calendar breaks on firefox when it opens
What are the steps to reproduce?
1- Go to the https://material.angular.io/components/datepicker/examples in firefox and open browser console
2- Open datepicker.
2- ERROR Error: "Invalid date "1" for month with index "9".". in console
Which versions of Angular, Material, OS, TypeScript, browsers are affected?
Firefox Quantum 61.0.1 (64-bit)
Ubuntu canonical - 1.0

Works with Firefox 62.0.2 (64-Bit) on Windows 10

I wasn't able to reproduce it either. What is your computer's locale?

Locale from the terminal:
LANG=en_US.UTF-8
LANGUAGE=
LC_CTYPE=en_US.UTF-8
LC_NUMERIC=en_US.UTF-8
LC_TIME=en_US.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=en_US.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=en_US.UTF-8
LC_NAME=en_US.UTF-8
LC_ADDRESS=en_US.UTF-8
LC_TELEPHONE=en_US.UTF-8
LC_MEASUREMENT=en_US.UTF-8
LC_IDENTIFICATION=en_US.UTF-8
LC_ALL=

@eslammahgoub It looks like this is working in Firefox Quantum 63.  Can you confirm that you are still seeing this in the latest versions?

yes still not working ERROR Error: "Invalid date "1" for month with index "10"."

@eslammahgoub have you figured it out?

it's working now I think it something relates to firefox


Forked process not killed after build or after specified task

I'm attempting to use this plugin to start a few servers necessary to run full browser tests. I can successfully start both servers, and move on to the tests, but after the tests are run, the servers remain active. I noticed a similar issue in #1 but the suggestion to set org.gradle.daemon=false didn't work.
Here is an example gradle task that I'm defining. Based on the documentation around the stopAfter property, I assumed that the servers would be killed after the task is complete:
task runTests(dependsOn: ['startBackend', 'startFrontend']) {
  doLast {
    // run tests
  }
}

task startBackend(type: com.github.psxpaul.task.ExecFork) {
  commandLine = './my_jar.jar'
  workingDir = "$project.rootDir/api/build/libs"
  waitForOutput = "Started ApplicationKt"
}

task startFrontend(type: com.github.psxpaul.task.ExecFork) {
  commandLine = './start.ui.sh'
  workingDir = "$project.rootDir"
  waitForOutput = "Compiled successfully"
}

Since the above wasn't working, I attempted to specify when the servers should be stopped using the stopAfter property. Again, after the task completes, the servers are both still running.
task runTests(dependsOn: ['startBackend', 'startFrontend']) {
  doLast {
    // run tests
  }
}

task startBackend(type: com.github.psxpaul.task.ExecFork) {
  commandLine = './my_jar.jar'
  workingDir = "$project.rootDir/api/build/libs"
  waitForOutput = "Started ApplicationKt"
  stopAfter =runTests
}

task startFrontend(type: com.github.psxpaul.task.ExecFork) {
  commandLine = './start.ui.sh'
  workingDir = "$project.rootDir"
  waitForOutput = "Compiled successfully"
  stopAfter = runTests
}

I've been able to kill the servers myself by finding the pid based on the port each server runs on and killing it. However, based on the documentation provided, I was under the assumption that I wouldn't need to do this. It's entirely possible I have something configured incorrectly, but figured I'd post in case others run into the same issue. For reference, I ended up killing the servers with lsof -i tcp:${port} | awk 'NR!=1 {print $2}' | xargs kill

I have the same problem. unfortunately, the .sh has some complex logic which should be tested, so I cannot get rid of it. I would need a way to kill child processes along with the actual process..

I've added a test for this, but it seems to work for me. From googling around, this might be a platform specific issue - what OS and shell are people using when they encounter this issue? I've tested this on MacOS with bash 3.2.57, as well as Ubuntu with bash 5.0.3.

We're using gradle start scripts for a Java based server (.bat, shell script), interesting enough:

It's fully reproducible on Windows. It happens always that after the stop task, the .bat is gone, but the java process lives on.
Yesterday when I commented, it was fully reproducible on Fedora 29, bash 4.4.23. I then started to try and grab the process handle via all kinds of illegal tricks from the task ;) (btw. would be nice to have "semi-official" access to it). I was dissatisfied with the results (was not working xD), so I hard-resetted back to where I started - and bam, it's no longer reproducible. I have /no/ idea what happened, what changed (nothing IMHO), but it works (again) on linux now...

I did some more googling and indeed the suggested method to make sure that child processes are destroyed as well is exactly the code I posted above, or put a little differently, this:
p.descendants().forEach(ph -> {
    ph.destroy();
});
p.destroy();

Could you maybe create an option to use this code instead of a plain single destroy? While at it, I would still appreciate a way to grab the actual process handle from the gradle script, so one could work around such issues in the meantime... :)

The project that prompted my original post has taken a new approach, so I can't guarantee that my current environment would still encounter the issue. However, I'm also on MacOS (10.14.6 fwiw) and bash 3.2.57. Based on the reactions to the original post, I assume others have run into the issue and could provide better help :P
If the issue appears to no longer be encountered, I'm fine with closing this based on your testing @psxpaul. Thanks for looking into this by the way!

Thanks for your feedback @mduft. Unfortunately I don't have access to a windows machine, so I can't confirm any fixes will work for you. If you want to try the code snippet you posted, you can add it to AbstractExecFork.stop(). I have already added a test case for it in sample_projects/shell_script. After running a ./gradlew clean build, you should see that no processes are still running and writing to sample_projects/shell_script/build/daemon.log.
I also imagine you'd want to recursively destroy all descendants, rather than just the first level.

Thanks for the suggestion, will try it ASAP. descendants() is already deep, children() would give use just direct children.

Is it acceptable to require Java 9? Otherwise the ProcessHandle API is not yet there and I guess it would be hard to call it xD I would have to use reflection to allow fixing the issue on 9+ while keeping <9 as is...

That's a really good question. We still use jdk 8 quite a bit at my company, and I imagine that's true at a lot of places. The reflection approach sounds acceptable to me, if it's not too difficult.

I thought a little more on this, and I think the reflection approach is not such a wise idea: The problem is that the plugin will magically behave different on Java <9 and 9+ - something that I very much dislike in day-to-day work :) I can still try to come up with a reflective approach pull request as well if you want to.

Aaaand here we go: #31
You may now choose any of the three pull requests :) #30 may even make sense with one of #29 or #31 applied, as it simply exposes the process handle...

@psxpaul Did you have a few minutes already to look at those pull requests?

Hey thanks for the contributions @mduft! Sorry for the delay, but this is a busy time of year for me.
I think #31 looks good, but I do agree that different behavior between Java <9 and 9+ is not ideal. I'm thinking of merging that PR and adding a new killDescendants option (defaulting to true). If killDescendants==true and Java <9, then error out. That way, users on Java 8 can set killDescendants=false to maintain current behavior, but users on Java 9+ will see the new behavior by default.

I have published version 0.1.13 that adds the killDescendants option, enabled by default. Please let me know if this fixes the issue or not.

Thats great, thanks! I will test ASAP.

I tested it, and it works perfectly fine on Linux and Windows :) Thanks again! For me, this issue is solved.


Window transparency

Hi,
Is there a way to enable notepad transparency in <IP_ADDRESS> version like it was in <IP_ADDRESS>? App loses its visual charm without it :(
Great job with fixing ANSIs though 👍
Cheers

Currently there is no way to restore the transparency in the TextBox, outside of directly changing the <ScrollViewer x:Name="svContent"                           Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
to
<ScrollViewer x:Name="svContent"                           Background="{ThemeResource SystemControlAcrylicWindowBrush}"
in the code itself. Reason for this change was to make to content pop-out more by providing a more clear contrast with the items that lay underneath it. (And with the logic that the more layers the app has, the less transparent/acrylic the layers will become 😉 )
Someone else also made a request to look into making black in the dark theme a bit more dark gray instead, so I'll take both this and the other piece of feedback into account when looking into improving that. :)


Bzz compression does not work due to bug in BSOutputStream or ZPCodec encoder implementation

See failing test DjvuNet.Compression.Tests.BzzWriterTests.BzzwriterTest001 - currently skipped in CI

Fixed with #54


Correct find syntax to fix grootfs cleanup

The find syntax is incorrect for the wildcard, causing cleanup to fail - https://github.com/cloudfoundry-incubator/kubecf/issues/1224

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):
Types of changes

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

Checklist:

[ ] My code has security implications.
[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.

I'm curious why the script is using find to delete the directory to begin with:
find /var/vcap/data/grootfs/ -iname '*' -delete

Why is it not simply
rm -rf /var/vcap/data/grootfs/

@mook-as Do you have any idea?
@viovanov Is this somehow related to the PVC mount problem?

@jandubois yes and in fact we located code in SCF where it is an rm -rf : https://github.com/SUSE/scf/blob/master/container-host-files/etc/scf/config/scripts/cleanup-garden-graph.sh#L4

Yeah, the find method was added by @viovanov in 3d05629 as part of #489 (which I suspected when I asked above about the PVC mount issues).
In https://github.com/cloudfoundry-incubator/kubecf/pull/489#issuecomment-600383822 I even quote the old SCF code that does the rm -rf, so I'm not sure whay Vlad changed it.
One difference is that the find code also removes the grootfs directory itself, whereas the rm -rf only removes all files (but not hidden files) from within. But if the goal was to removed everything include the dir itself, then you only have to strip the trailing * from the rm -rf command, so I still don't understand.
We need to wait until @viovanov can chime in.

Unfortunately I don't remember. It's possible I started with a pattern, and it evolved to *.
If the new syntax works, I'd leave it as is.

If the new syntax works, I'd leave it as is.

I agree that it is not terribly important, but if the more obvious rm -rf ... works as well, then I would prefer to merge the simpler form. Otherwise people will wonder if they ever look at this why the complex expression is necessary.
I guess it depends on @univ0298's willingness to try and test the rm -rf ... form as well. Otherwise, I'm also fine with merging as-is.

We could try the rm approach fairly easily I think. But it seems quite surprising that @viovanov would have switched from rm to find, without any reason

We could try the rm approach fairly easily I think.

I would appreciate if you could do that.

But it seems quite surprising that @viovanov would have switched from rm to find, without any reason

I agree. Which makes it even more important to add a comment to explain why this command is necessary, and how it is different from a plain rm -rf.
I don't think it is though: The term -iname * doesn't make much sense, it should be -name *, but that still is a condition that is always true, so you could just write find /var/vcap/data/grootfs/ -delete. And now it is even less clear how this could possibly be different from rm -rf /var/vcap/data/grootfs/.
I suspect that this was just a random/desperate change (that didn't actually make any difference) while trying to get the nested PVC mounting to work, which took several days. And when it was finally working, the inconsequential changes just remained in place to avoid having to go through even more testing cycles.

@jandubois I tried with the rm -rf alternative and it also worked just fine, so you could amend to that. I simply remain a bit suspicious that @viovanov had some mysterious good reason to deviate away from that in the first place! But it seems fairly clear that rm would be equally safe and cleaner


Aalcesto23

Looks like you've got commits mixed in from the previous exercise. I'll take a look at this one in a moment, but make sure to keep a clean branch without prior commit history.

Hey Joe,
Sorry about that, I’ll make sure to use a new branch moving forward. On a side note, I’m working on the Robot Name challenge and having some trouble. Would it be possible to submit what I’ve got and get some feedback?
Regards,
Alan

On May 26, 2016, at 12:27 PM, Joseph Mastey<EMAIL_ADDRESS>wrote:
Looks like you've got commits mixed in from the previous exercise. I'll take a look at this one in a moment, but make sure to keep a clean branch without prior commit history.
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub https://github.com/jmmastey/level_up_exercises/pull/1174#issuecomment-221938610

Yeah, absolutely.
Joseph Mastey
@jmmastey http://twitter.com/jmmastey|<EMAIL_ADDRESS>|<PHONE_NUMBER>
| http://josephmastey.com | http://leveluprails.com
On Thu, May 26, 2016 at 12:31 PM, Alan Alcesto<EMAIL_ADDRESS>wrote:

Hey Joe,
Sorry about that, I’ll make sure to use a new branch moving forward. On a
side note, I’m working on the Robot Name challenge and having some trouble.
Would it be possible to submit what I’ve got and get some feedback?
Regards,
Alan

On May 26, 2016, at 12:27 PM, Joseph Mastey<EMAIL_ADDRESS>wrote:
Looks like you've got commits mixed in from the previous exercise. I'll
take a look at this one in a moment, but make sure to keep a clean branch
without prior commit history.
—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub <
https://github.com/jmmastey/level_up_exercises/pull/1174#issuecomment-221938610

—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/jmmastey/level_up_exercises/pull/1174#issuecomment-221939420

Cool, commented.  Good start.
Reopen with separate branches for the different exercises. Take a look at git cherry-pick for more on how to do this. Basically you need a clean master branch, then make a topic branch and cherry-pick in the relevant commits.
👍


Replace YAML package definitions with standard package.json, fix dependencies

Included in this:

removed yapm
replaced package.yaml with standard package.json
removed post-install reconstruction of npm-shrinkwrap file, which defeats the purpose of the file
removed JSON5 body parser (four year old proposed extensions to JSON which have not yet happened) and reinstated standard Express body parser
reverted Express 5 alpha to Express 4 for maximum stability
replaced all out of date version dependencies in the new package.json with the latest versions to clean up the warnings

The motivation is to become more standards-compliant to assist contributors, and bring all dependencies up to the latest stable release.
Note: I have not changed the author section in package.json, which needs attention afterwards. I have left the licence as WTFPL which I'm not personally a fan of, but perhaps merits a later debate.

BTW, I'd be interested to know if this fixes #5 for you. It does for me (v4.4.3), so you never know.

Awesome this look great, I should have time to look at this by end of day

I noticed that the shrink-wrapped URL for the symbol package is coming from localhost:4873 so I wasn't able to do a clean npm install without removing npm-shrinkwrap.json first.
Hi, by the way! I'm considering using verdaccio at my company. I'd be willing to contribute to the project from time to time.

Well spotted! I will fix it and redo the request.

Rather than reverse the whole thing again, I just added another commit to fix the file in question. I remembered to reset the registry and npm clear cache before re-installing and generating the shrink-wrap file this time. I reran the tests too - all pass.
@trentearl let me know if you need to me to do the whole thing as one commit. I've seen other projects where people only want single commit merge requests, although I'm not sure what difference it makes.
@andrewjensen more contributors are always welcome, and you should find Verdaccio is a good choice. I've got about a dozen servers and a couple of dozen users pointed at ours, and it works great, both with our own private locally scoped packages and cached public ones.

@steve-p-com There's actually a new feature in Github where you can squash a PR into a single commit as you merge it using the web GUI! I would say use that.

@steve-p-com Sorry for the hassle but I would prefer one commit. It makes the history a lot easier to read and makes it a lot easier to rollback. Also you'll notice the two commits here are by separate users.
Once thats done i'll merge it 👍  thanks for this, this is the number one thing I wanted!

@andrewjensen Great catch, yeah welcome aboard!


Add uv script to pyenv's pip-rehash

See https://github.com/pyenv/pyenv/tree/master/pyenv.d/exec/pip-rehash
This is used to recalculate the environment on installation and there are shims for pip, conda, and easy_install.
This will resolve https://github.com/astral-sh/uv/issues/1586
Tracking in pyenv at https://github.com/pyenv/pyenv/issues/2979

They've suggested we should provide a pyenv plugin that adds this functionality.


Running the binaries

My bad, I probably damaged the files structure.
Anyway- do you have any idea how the files were compiled such that

RPATH is shown on on checksec
C code file (when avialable) is shown

the answer was quite simple

use linker flags
use -g as an argument to gcc / CFLAGS (keep debug symbols)

something like this
CFLAGS=-g -Wall -no-pie -Wextra -fstack-protector-all -D_FORTIFY_SOURCE=2 -Wl,-z,now -Wl,-z,relro,-z,noexecstack
LDFLAGS=-Wl,--rpath=../../.glibc/glibc_2.31/ -Wl,--dynamic-linker=../../.glibc/glibc_2.31/ld.so.2

.glibc directory is too heavy for GitHub, I've gitignore'd it. Without it, the binaries won't run, obviously.
Yep, that's correct. The C code you see in pwndbg is mapped to assembly one using debug symbols.
Anyway, @IdanBanani , next time it's better to ask Max these questions, he is the instructor after all, not me.

@IdanBanani can you write all the commands that will let me run the binaries?


webcat: init at unstable-2021-09-06

Motivation for this change
Things done

Built on platform(s)

[x] x86_64-linux
[ ] aarch64-linux
[ ] x86_64-darwin
[ ] aarch64-darwin

[ ] For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[x] Tested execution of all binary files (usually in ./result/bin/)
21.11 Release Notes (or backporting 21.05 Release notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[x] Fits CONTRIBUTING.md.

Hi @Pacman99,
Thx for the review, I have made all the changes you have suggested in c786f97. Hope it's OK for you now :smiley:

@figsoda I've made all the changes requested! Thx for your review :+1:

Result of nixpkgs-review pr 140202 run on x86_64-linux 1

1 package built:

webcat