Handle flat data at the beginning of the recording

Fixes https://github.com/cbrnr/sleepecg/issues/87
Flat data at the beginning of the ECG recording can mess up the Pan & Tompkins heartbeat detection. This PR adds a fix for that, such that the heartbeat detection only starts after the first non-flat sample of the data. The index of the first non flat sample is then added back to the array of heartbeat indices.
The overhead of calculating np.nonzero(np.diff(ecg))[0] is ~100 ms for a 8 hour ECG signal sampled at 256 Hz, on a 2019 MacBook Pro:
%timeit np.nonzero(np.diff(np.random.rand(256 * 8 * 3600)))

I have also added unit tests and updated the changelog.
Feel free to edit,
Thanks,
Raphael

There are some style issues. Also, 100ms is not much in absolute time, but it almost doubles the detection time for the C and Numba backends. I'd rather not do this by default, but maybe introduce a parameter (which by default does not detect flat segments). Not sure what a good name for this parameter would be though.

I like the explicit condition better.

Done — I have added a unit test to catch the ValueError. Let me know if you need anything else from me. Thanks

Thanks! If possible, I'd add a more specific test for the ValueError (so include at least part of the message). Other than that, this looks great! I will do my benchmarks to see how this performs, and then we can decide on having a new parameter or not.

@LegrandNico could you test if this PR fixes the issues you had with one of your data sets?

@raphaelvallat currently we set all samples before the first zero-crossing to 0:
https://github.com/cbrnr/sleepecg/blob/main/sleepecg/heartbeats.py#L128-L131
I wonder (1) if we can use this step also for detecting flat segments and (2) why setting 2s of flat data is OK (better?) than longer flat segments.
@hofaflo maybe you have some insights?

@LegrandNico could you test if this PR fixes the issues you had with one of your data sets?

Yes the function is working correctly now 👍

Hi here, is there any plan to merge and release this PR? Thanks!

Sorry that it's taking so long. Yes, I will merge this PR soon (today?). A new release is also planned sometime soon, but probably not within the next 3-4 weeks since I will be on vacation then.

Thanks @raphaelvallat!


fetchIdToken fails with a 400 error when using the Compute client on GKE with Workload Identity

Environment details

OS: Alpine Linux v3.11
Node.js version: v14.18.1
npm version: 6.14.15
google-auth-library version: 7.9.2

Steps to reproduce

Run a container in GKE with Workload Identity configured
Attempt to generate an ID token using the following code:

const google = require('google-auth-library');
const client = await new google.GoogleAuth().getClient()
console.log(await client.fetchIdToken('IAP_CLIENT_ID'))

Uncaught:
GaxiosError: Could not fetch ID token: Unsuccessful response status code. Request failed with status code 400
    at Gaxios._request (/app/node_modules/gaxios/build/src/gaxios.js:129:23)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async metadataAccessor (/app/node_modules/gcp-metadata/build/src/index.js:68:21)
    at async Compute.fetchIdToken (/app/node_modules/google-auth-library/build/src/auth/computeclient.js:80:23)
    at async REPL4:1:36 {
  response: {
    config: {
      url: 'http://<IP_ADDRESS>/computeMetadata/v1/instance/service-accounts/default/identity?format=full&[SNIP].apps.googleusercontent.com',
      headers: [Object],
      retryConfig: [Object],
      responseType: 'text',
      timeout: 3000,
      paramsSerializer: [Function: paramsSerializer],
      validateStatus: [Function: validateStatus],
      method: 'GET'
    },
    data: 'non-empty audience parameter required\n',
    headers: {
      connection: 'close',
      'content-length': '38',
      'content-type': 'text/plain; charset=utf-8',
      date: 'Tue, 19 Oct 2021 10:50:12 GMT',
      'x-content-type-options': 'nosniff'
    },
    status: 400,
    statusText: 'Bad Request',
    request: {
      responseURL: 'http://<IP_ADDRESS>/computeMetadata/v1/instance/service-accounts/default/identity?format=full&audience=[SNIP].apps.googleusercontent.com'
    }
  },
  config: {
    url: 'http://<IP_ADDRESS>/computeMetadata/v1/instance/service-accounts/default/identity?format=full&audience=[SNIP].apps.googleusercontent.com',
    headers: { 'Metadata-Flavor': 'Google' },
    retryConfig: {
      noResponseRetries: 3,
      currentRetryAttempt: 0,
      retry: 3,
      httpMethodsToRetry: [Array],
      statusCodesToRetry: [Array]
    },
    responseType: 'text',
    timeout: 3000,
    paramsSerializer: [Function: paramsSerializer],
    validateStatus: [Function: validateStatus],
    method: 'GET'
  },
  code: '400'
}

Cause of issue
After some debugging what is going wrong it seems the Workload Identity Metadata server only accepts the audience query parameter if it is the first argument.
# Returns a valid token
$ wget -qO- --header="Metadata-Flavor: Google" http://<IP_ADDRESS>/computeMetadata/v1/instance/service-accounts/default/identity?audience=[snip].apps.googleusercontent.com&format=full
eyJhbGciOiJSUzI1NiIs[...snip...]

# Returns the same 400 error, note that only the order of the query params changed
$ wget -qO- --header="Metadata-Flavor: Google" http://<IP_ADDRESS>/computeMetadata/v1/instance/service-accounts/default/identity?format=full&audience=[snip].apps.googleusercontent.com
wget: server returned error: HTTP/1.1 400 Bad Request

@sjaq thanks for digging into this, this is a somewhat fascinating bug.
@bojeil-google is this a known issue with the workload identify server, I'm wondering if we should open an internal bug.

Thanks for looking into this!
Is this something that I need to work around in my implementation or something I can expect an update of the library for? Just want to prevent any unnecessary workaround in my codebase... Seems like an easy fix to reorder the query arguments in the library itself?

Hey @sjaq, we will implement it the same way I am describing above (it is not a temporary fix or hack). So you can do it on your own if you can't wait. We want to eventually do it but we have to align across languages (we need to support in several languages and not just Node.js), so it may take time to get to it given the many projects in our pipeline at the moment.

@bojeil-google I'm using workload identity as described here to run workloads that have a provided Google Service Account, when I use the nodejs client I'm provided with a Compute client as the default authorization client, not sure what I could do differently on my end?

@sjaq I'm glad you've found a workaround that's doing the trick for you.
It seems like a nice feature would be if you could somehow upgrade a Compute client to a WorkLoadIdentityCompute client? Leaving this open as a feature request.


LibreOffice doesn't respect Dark Mode except for icons

Describe the bug
Fresh bluefin-dx install. Chose LibreOffice at the welcome wizard. Enabled GNOME dark mode. Opened LibreOffice apps, but all of them are light. Icons are also light so it looks broken.

What did you expect to happen?
Dark LibreOffice apps with matching icons
Output of rpm-ostree status
State: idle
AutomaticUpdates: stage; rpm-ostreed-automatic.timer: last run 1h 45min ago
Deployments:
● ostree-image-signed:docker://ghcr.io/ublue-os/bluefin-dx:latest
                   Digest: sha256:b63ee00bb13ed24423bf4ea942d80793c9ef68815653ab897183738140747210
                  Version: 39.20240125.0 (2024-01-25T16:53:47Z)

  ostree-unverified-image:docker://ghcr.io/ublue-os/bluefin-dx:latest
                   Digest: sha256:b63ee00bb13ed24423bf4ea942d80793c9ef68815653ab897183738140747210
                  Version: 39.20240125.0 (2024-01-25T16:53:47Z)

Extra information or context
In light mode the icons are normal.
Silverblue does not have this bug.

Please change your GTK Theme using Gnome Tweaks to something like Adwaita or adw3-gtk and make the sure the corresponding version is installed in flatpak.
It appears that flatpak did not install a corresponding dark theme automatically for you using the default Yaru-blue theme.

Yes, setting the theme for legacy applications to Adwaita fixes this (without installing anything). So it seems like yaru-blue simply doesn't provide a dark theme for LibreOffice.


[BUG]

Information

Apktool Version (apktool -version) - latest 2.9.0
Operating System (Mac, Linux, Windows) - windows
APK From? (Playstore, ROM, Other) -  happymod
Java Version (java --version) - 18.0.2

o sistema não pode encontrar o arquivo especificado = the system can't find said file

Steps to Reproduce

apktool b
apktool b

Frameworks
If this APK is from an OEM ROM (Samsung, HTC, LG). Please attach framework files
(.apks that live in /system/framework or /system/priv-app)
APK
If this APK can be freely shared, please upload/attach a link to it.
https://happymod.com/pou-122-7-mod/me.pou.app/original-download.html
Questions to ask before submission

Have you tried apktool d, apktool b without changing anything? yes
If you are trying to install a modified apk, did you resign it? no
Are you using the latest apktool version? yes

Hmm. I couldn't replicate locally.
➜  3424 apktool d 3424.apk -s -f
I: Using Apktool v2.9.0-8-34a69708-SNAPSHOT on 3424.apk
I: Loading resource table...
I: Decoding file-resources...
I: Loading resource table from file: /home/ibotpeaches/.local/share/apktool/framework/1.apk
I: Decoding values */* XMLs...
I: Decoding AndroidManifest.xml with resources...
I: Regular manifest package...
I: Copying raw classes.dex file...
I: Copying raw classes2.dex file...
I: Copying assets and libs...
aI: Copying unknown files...
I: Copying original files...
➜  3424 apktool b 3424
I: Using Apktool v2.9.0-8-34a69708-SNAPSHOT
I: Copying 3424 classes.dex file...
I: Copying 3424 classes2.dex file...
I: Checking whether resources has changed...
I: Building resources...
I: Copying libs... (/kotlin)
I: Building apk file...
I: Copying unknown files/dir...
I: Built apk into: 3424/dist/3424.apk

Must be something directory specific - could you try using the linux subsytem on Windows? That'll help pinpoint.


ignore rules for Microsoft Visual Studio Community 2022

@LGUG2Z I have expanded on the ignore rules for Visual Studio.
I am not 100% sure if the composite rules need to include the match on Exe
{
  "kind": "Exe",
  "id": "devenv.exe",
  "matching_strategy": "Equals"
}

But let's see what you say 🤔

Looks good! I think when matching on Title it's good to use a composite rule with Exe to reduce the blast radius.

@CtByte @LGUG2Z Couldn't these ignore windows be set to floating? As far as I understand these are some sort of popup windows that need to be floating right? Or are they internal windows of visual studio that stay inside it when ignored?
Because if they are floating windows, we could use the floating so that they are still bound to the same workspace instead of showing on all workspaces like they do when ignored...

@alex-ds13 From what I remember when adding these rules, most of these are for loading windows, search windows, confirmation popups and such.
I would not mind if they are floating, I just did not think these should be managed at all.
If they are managed, would they also have the "komorebi-drawn" borders?

If they are managed, would they also have the "komorebi-drawn" borders?

If you want to they can. Or you can set the floating border to 0x000000 which will make them invisible. In that case the only difference would be that if you changed workspace while one of those windows was opened, then that window would be "cloaked"/"hidden" like all other managed windows and when you move back to that workspace again it would show up again...

I can see the benefit in that, it might feel cleaner if the popups belong to the workspace. One of the windows is a "transparent placeholder" so at least that should be ignored. The confirmation and popup windows could be floating.
But let's see what Jeezy thinks.

My gut feeling is:
If it only gets popped up to be immediately clicked away (confirmations etc.) -> ignore rule
If it gets popped up, stays open and is interacted with for a while before being closed -> floating rule
But I'm open to having my mind swayed!

ok, so since I could not see any floating rules in the json file, would this be the correct syntax?
  "Visual Studio": {
    "object_name_change": [
      {
        "kind": "Exe",
        "id": "devenv.exe",
        "matching_strategy": "Equals"
      }
    ],
    "floating": [
      // floating rules
    ],
    "ignore": [
      // ignore rules
    ]
  },

EDIT: I will test my application.json locally first to see if having the border for floating windows is annoying 😅 I will come back with an update at the end of my workday

You can always make the borders invisible like I've mentioned before. Also I've noticed that when using floating windows, when they lose focus the border seems to be removed and it only shows up again if you move or resize the window...

I've found the floating windows having the border to be nice, if I am honest, but the border disappears for me too when the window lost focus.
https://github.com/user-attachments/assets/5a966424-efe8-4e9d-894b-e413d17e0b5a


Fix false alerts about uninstalled packages

https://trello.com/c/5JpkmLwB/940-fix-not-alerting-about-outstanding-security-updates
Please see this commits for more details.

Thanks @ChrisBAshton! I've fixed that, and added another commit to a dev doc to try and help future people. Could you take another look at this and that?


Wrong previous issue is closed

In one of my workflows, I have a step looking like that:
      - name: Run tests
        run: ...

      - name: Open issue
        if: failure()
        uses: imjohnbo/issue-bot@v2
        with:
          title: CI Failure
          labels: bug, ci
          pinned: true
          close-previous: true
          template: .github/templates/ci-failure.md

When tests are failing, then issue-bot creates an issue. However, it also closes the last issue with a bug label. Not with a bug AND ci label. Only with a bug label.
I would expect issue-bot to close the last created issue with these two labels.

Thanks for the report, @arugifa! I will try to take a look but would welcome a PR in the meantime.
Note to future self or other contributors: investigate whether GraphQL labels are AND or OR here.

@arugifa, a fix is now available 🥳.
You're welcome to test on the release/v3 branch before the v3 release is available next week. See more here: https://github.com/imjohnbo/issue-bot/pull/33.

Amazing @imjohnbo ! Thanks a lot :partying_face: :tada:


Sol003 driver- Drivers vulnerabilities fixes

Issue: #41
Description of the changes: #41
List of related PRs. : None
Project builds without any errors.
Unit Tests updated (or created where needed) and all pass.
You have ran manual functional “smoke” test (does product as a whole still work?).
User Story meets the acceptance criteria - and passes. acceptance criteria should be understood at outset.
There is a PR for update of internal documentation for the affected Project(s) - README etc.

Hi @hhellyer , @owen-lynch-ibm , Could you please review this PR for the vulnerabilities fix.

Hi @hhellyer : Could you please merge the changes and close this PR.


Outdated soucecodes to actual sourcecodes in Angular documentation

Bug, feature request, or proposal:
Proposal
What is the expected behavior?
Actual sourcecodes in Angular Material documentation https://material.angular.io/components/
What is the current behavior?
Outdated sourcecodes
What are the steps to reproduce?
Replace md to mat
What is the use-case or motivation for changing an existing behavior?
It confuses angular beginners
Which versions of Angular, Material, OS, TypeScript, browsers are affected?
latest Angular Material
Is there anything else we should know?
no

Did you clear your cache?

See #8497
Visit chrome://serviceworker-internals/ and unregister the worker for material.angular.io


Fix failure to delete jwt cookie on logout

For some reason the jwt cookie is not being deleted on logout.
From what I could gather it is because response.delete_cookie() is not generating the same cookie as the dj_rest_auth login view (differing httponly and samesite values).
This has been fixed and does not break any of the existing tests (run on tox)
Also fixed issue where "Neither cookies nor..." message was being sent even when jwt_cookie is set

My cookies are correctly deleted.
You probably received the message because you didn't add the blacklist app to your installed app.
https://github.com/jazzband/dj-rest-auth/blob/06eeac95e0499640bf10f30a13fabd5ae33f73d8/dj_rest_auth/views.py#L193
But I agree that the current state of the logout is a bit confusing, it doesn't make a lot of sense to receive an "error" message with a 200.
But I think it's a really bad idea not to implement a blacklist or use the one given by simplejwt package. Without blacklist your token could still be used for a long time after you logout!
From what I understand, I feel like this error message was more there to warn devs, but it's not the right way of doing that, because the final user will also receive that message in the request and it doesn't make sense for the user to receive this kind of messages.
Oh and btw the message is not that usefull anyway because if you have set the cookie in httpOnly there is no way to remove the cookie via the JS. And you can't ask the final user to go into the browser debug and clean the cookies.
In my opinion and from what I understood, this else condition makes no sense, and should simple be removed.
Instead there should have been the following code right after token.blacklist() :
response.status_code = status.HTTP_200_OK
And a warning in the documentation of this package to warn the devs to use the blacklist app for security reason or warn them that the refresh cookie could be used even after logout if there is no blacklist mecanism.
Hope it helps ;)

My cookies are correctly deleted.
You probably received the message because you didn't add the blacklist app to your installed app.
https://github.com/jazzband/dj-rest-auth/blob/06eeac95e0499640bf10f30a13fabd5ae33f73d8/dj_rest_auth/views.py#L193
But I agree that the current state of the logout is a bit confusing, it doesn't make a lot of sense to receive an "error" message with a 200.
But I think it's a really bad idea not to implement a blacklist or use the one given by simplejwt package. Without blacklist your token could still be used for a long time after you logout!
From what I understand, I feel like this error message was more there to warn devs, but it's not the right way of doing that, because the final user will also receive that message in the request and it doesn't make sense for the user to receive this kind of messages.
Oh and btw the message is not that usefull anyway because if you have set the cookie in httpOnly there is no way to remove the cookie via the JS. And you can't ask the final user to go into the browser debug and clean the cookies.
In my opinion and from what I understood, this else condition makes no sense, and should simple be removed.
Instead there should have been the following code right after token.blacklist() :
response.status_code = status.HTTP_200_OK
And a warning in the documentation of this package to warn the devs to use the blacklist app for security reason or warn them that the refresh cookie could be used even after logout if there is no blacklist mecanism.
Hope it helps ;)

LGTM 👍

LGTM 👍


KAFKA-14908: Set setReuseAddress on the kafka server socket

Changes SocketServer to set the setReuseAddress(true) socket option.
This aids use-cases where kafka is started/stopped on the same port in rapid succession. Examples are: where a kafka cluster is embedded in an integration test suite that starts/stops a cluster before/after each test.
I don't think this change brings any backward compatibility concerns, so I don't think it will need to be configurable.  However I can make it that way if necessary.
I have tested this change locally and confirmed that with the option the sporadic issue I see goes away.
Committer Checklist (excluded from commit message)

[ ] Verify design and implementation
[ ] Verify test coverage and CI build status
[ ] Verify documentation (including upgrade notes)

@divijvaidya thanks for taking time to put together the detailed reply, I'll make the changes you are suggesting.

@divijvaidya , thanks for the comment. I agree with you most of them. But for this point:

I would suggest to extract out a method "createSocketServerChannel()" in SocketServer.java. This method would create the channel and set the configuration on the socket. Use this same method in all the tests.

Since the SelectorTest and other network tests are in client module, we should not add core dependency to client module. So, I think adding a util method in TestUtils should be good enough. WDYT?

@divijvaidya , thanks for the comment. I agree with you most of them. But for this point:

I would suggest to extract out a method "createSocketServerChannel()" in SocketServer.java. This method would create the channel and set the configuration on the socket. Use this same method in all the tests.

Since the SelectorTest and other network tests are in client module, we should not add core dependency to client module. So, I think adding a util method in TestUtils should be good enough. WDYT?

NioEchoServer,  SelectorTest and SslSelectorTests  already make sensible use of bind(0), so the listening sockets they create cannot actually suffer the problem described in the JIRA, even if those tests were run in tight loop.  The network stack  would chose an ephemeral port that was not in timed wait.    I think there is an argument for leaving these tests untouched.
I'm +1 to adding a new core test to ensure that the listener sockets made by Acceptors  do have SO_REUSEADDR set correctly and of course the comment to explain why it is so.

@showuon That's a good point. The problem I want to solve here is to ensure that tests are using the same code that SocketServer is using. If we implement a method in TestUtils then the tests may not catch bugs when SocketServer was modified but TestUtils wasn't. But I agree with your concern about taking a dependency on core.
I agree with ignoring the part of my suggestion to modify tests (suggestion 2 in my previous comment) for now. But let's keep the part about adding a unit test to verify that this configuration was added (suggestion 4 in my previous comment).

@divijvaidya thanks for the review feedback, much appreciated.

Hi all,
Unfortunately I had to revert this. This is a change to our public API since it will allow multiple Kafka servers to be started on the same port. This kind of change needs a KIP (Kafka Improvement Proposal) so we can discuss the pros and cons.
The change also does very different things on different operating systems. There is a rundown here: https://stackoverflow.com/questions/14388706/how-do-so-reuseaddr-and-so-reuseport-differ

Security: SO_REUSEADDR can allow an attacker to perform DDOS by creating a genuine connection with same IP and port. Prior to this change, the TIME_WAIT state of the socket would have prevented immediate re-connection. This is an acceptable risk for Kafka because we have connection throttling available in code for IP addresses and a user may choose to configure it to prevent a DDOS.

Again, this needs a KIP so we can discuss whether the increased DDOS risk is acceptable or not. The answer may be different for different users.
Personally, I think that at minimum SO_REUSEADDR should be an option (not mandatory) and probably not the default.

Hey @cmccabe, we can have a community discussion over the mailing list but I am not following one of your statements here.

since it will allow multiple Kafka servers to be started on the same port

Can you please help me understand why? I am asking this because AFAIK (correct me if I am wrong), all that this flag does is to allow re-use of a socket which is in TIME_WAIT state. If it is in another state, i.e. it is being used by a different Kafka process, the binding is not allowed, hence, two Kafka servers cannot run on the same port (or rather in this context, reuse the socket).

Hello @cmccabe, thank you for the feedback.

since it will allow multiple Kafka servers to be started on the same port

Having read that SO article I now see the SO_REUSEADDR does indeed have that effect on some platforms.  It was not my intent to allow that. I was unknowingly fixating on the Linux behaviour.
For example, SO article discusses BSD behaviour:

Without SO_REUSEADDR, binding socketA to <IP_ADDRESS>:21 and then binding socketB to <IP_ADDRESS>:21 will fail (with error EADDRINUSE), since <IP_ADDRESS> means "any local IP address", thus all local IP addresses are considered in use by this socket and this includes <IP_ADDRESS>, too. With SO_REUSEADDR it will succeed, since <IP_ADDRESS> and <IP_ADDRESS> are not exactly the same address, one is a wildcard for all local addresses and the other one is a very specific local address. Note that the statement above is true regardless in which order socketA and socketB are bound; without SO_REUSEADDR it will always fail, with SO_REUSEADDR it will always succeed.

Personally, I think that at minimum SO_REUSEADDR should be an option (not mandatory) and probably not the default.

I'd be happy to resubmit the PR in this way.


Update structure name

Renamed some structures.
Please see: https://github.com/c9s/r3/issues/63

Thank you!
I haven't found this upstream change :sweat_smile:


Corrected admin.E013 check message in docs.

https://github.com/django/django/blob/d6e9ec40145b9edc65e8d0c65bd3f4ef8a7a27bb/django/contrib/admin/checks.py#L450-L454

Thanks both for checking :+1:


Notmuch-hello pops up in a side window rather than using the current one

What did you expect to happen?
I expected notmuch-hello to use the current window
What actually happened?
It doesn't, and instead opens in a small split window.
Additional details:
notmuch-hello uses pop-to-buffer-same-window, so I feel like it should be able to take the current window.
Could it be something in Doom's window switching hooks?
Also, any general advice on how to diagnose where issues are occuring (Emacs, notmuch, Doom) would be appreciated.
System information:

```
SYSTEM  type       gnu/linux
        config     x86_64-pc-linux-gnu
        shell      /bin/bash
        uname      Linux 5.10.10-arch1-1 #1 SMP PREEMPT Sat, 23 Jan 2021 23:59:48 +0000 x86_64
        path       (~/.emacs.d/bin/ ~/.emacs.d/bin ~/.ghcup/bin ~/dls/gcc2016q1/gcc-arm-none-eabi-5_3-2016q1/bin ~/.local/bin ~/.dynamic-colors/bin ~/.emacs.d/bin ~/.ghcup/bin ~/dls/gcc2016q1/gcc-arm-none-eabi-5_3-2016q1/bin ~/.local/bin ~/.dynamic-colors/bin ~/.emacs.d/bin ~/.ghcup/bin ~/dls/gcc2016q1/gcc-arm-none-eabi-5_3-2016q1/bin ~/.local/bin ~/.dynamic-colors/bin /usr/local/sbin /usr/local/bin /usr/bin /usr/lib/jvm/default/bin /usr/bin/site_perl /usr/bin/vendor_perl /usr/bin/core_perl ~/.go/bin ~/bin ~/.go/bin ~/bin /usr/lib/jvm/default/bin ~/.go/bin ~/bin /usr/lib/emacs/27.1/x86_64-pc-linux-gnu/)
EMACS   dir        ~/.emacs.d/
        version    27.1
        build      Aug 28, 2020
        buildopts  --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-wide-int --with-modules --with-cairo --with-harfbuzz 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt' CPPFLAGS=-D_FORTIFY_SOURCE=2 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now
        features   XPM JPEG TIFF GIF PNG RSVG CAIRO SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ M17N_FLT LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP
        traits     (gui server-running envvar-file)
DOOM    dir        ~/dots/emacs/.doom.d/
        version    2.0.9
        font       #
        theme      doom-monokai-pro
        build      HEAD -> develop b03fdabe4 2021-01-13 14:07:47 -0500
        elc-files  0
        modules    (:completion company ivy :ui doom doom-dashboard doom-quit hl-todo ophints (popup +defaults) vc-gutter vi-tilde-fringe workspaces zen :editor (evil +everywhere) file-templates fold format snippets :emacs dired electric undo vc :term vterm :checkers syntax spell :tools debugger (eval +overlay) lookup lsp magit pdf :lang (cc +lsp) common-lisp emacs-lisp (haskell +lsp) latex markdown (org +roam +noter) (python +lsp) racket (rust +lsp) sh :email notmuch :app (rss +org) :config (default +bindings +smartparens))
        packages   ((messages-are-flowing) (org-msg) (hledger-mode) (org-ql) (counsel-notmuch) (tickscript-mode) (org-super-agenda))
        unpin      (n/a)
        elpa       (n/a)
```

That's the default behaviour after an update, if you don't want the popup, then you can add this to your config:
(after! notmuch (set-popup-rule! "^\\*notmuch-hello" :ignore t))

That's the default behaviour after an update, if you don't want the popup, then you can add this to your config:
(after! notmuch (set-popup-rule! "^\\*notmuch-hello" :ignore t))

Perfect, thanks for that brongulus, guess I need to keep a closer eye on changenotes :)

Perfect, thanks for that brongulus, guess I need to keep a closer eye on changenotes :)


Markdown Import options

Made import options for markdown helper of that name. To standardise the interface I did it for exports to.
The added import options allows configuring of the empty lines behaviour on import.
I will add tests and change the documentation if the basic idea is accepted.

Closing this PR due to staleness! If there are new updates, please reopen the PR.


go/ast: provide AST Rewrite functionality (ast.Walk is not good enough)

This has come up again and again. While it's easy to traverse the AST (ast.Walk, ast.Inspect), it not easily possible to use those for general tree rewrites w/o significant amount of work.
For instance, if we wanted to rewrite all expressions (ast.Expr) of a certain kind into something else, ast.Walk and ast.Inspect would need to consider every node that contains ast.Expr fields separately, rather than just look at nodes that satisfy ast.Expr.
AST rewriters exist in other packages; most notably perhaps in gofmt (for gofmt -r); this one is reflection-based. Reflection-based approaches tend to be general, but also hard to understand, and slower than necessary.
API starting point:
// An ApplyFunc is invoked by Apply for each node, before and/or after
// the node's children. See Apply for the interpretation of the return
// value.
//
// The node is given by the node pointer's address addr which makes it
// possible for an ApplyFunc to rewrite the node. The node's parent is
// the node containing the *addr. If the node is part of a list, index
// identifies its position in that list. Index is < 0 otherwise.
type ApplyFunc func(parent Node, index int, addr interface{}) bool

// Apply traverses a syntax tree recursively, starting with the node
// identified by parent, index, and addr. See Apply for the meaning
// of these arguments.
//
// If pre is not nil, it is called for each node before its children
// are traversed (pre-order). If the result of calling pre is false,
// no children are traversed, and post is not called for the node.
//
// If post is not nil, it is called for each node after its children
// were traversed (post-order). If the result of calling post is false,
// Apply returns immediately.
func Apply(parent Node, index int, addr interface{}, pre, post ApplyFunc) {

cc: @alandonovan

This seems like a good addition.  The reflect-based implementations in gofmt (and eg) is fearsomely complex, and there are other times I have wanted to use this function.
The index is not enough to uniquely identify a subtree of a node.  For example, ast.CaseClause contains two slices of Nodes, and ast.BinOp contains two Exprs.  I think we need to identify the field too.  The most obvious way to do that is by its name.  Field numbers, or field offsets, might be marginally more efficient, but would certainly be harder to read, and I'm users would thank us for choosing strings when they're debugging.
I'm not sure you need the index (or index + field name) in the Apply function, only in ApplyFunc.

I've also wanted this.
Index is sufficient I think if it counts across all children.  E.g., in AssignStmt, it counts from 0 to len(Lhs)+len(Rhs)-1.
I've had cases where I want to be able to walk up the AST more than one Node, so I maintain a slice of (Parent, Index) tuples.
Aside: Index technically isn't necessary, and Java's Compiler API omits it from com.sun.source.util.TreePath.  But I think it's handy to have.
I'm not sure I understand what addr's dynamic type will be from the description.  I'm guessing though that if I'm visiting a *ast.Ident, then it could be either a **ast.Ident (e.g., the name for a ValueSpec) or *ast.Expr (e.g., an identifier in an expression context)?

The dynamic type would be a pointer to the variable (field or slice element), whatever that may be: Node, Expr, *Ident.

Here's a slightly more thought-through API that provides explicit names for fields. Nice, but definitively somewhat costly:
// An ApplyFunc is invoked by Apply for each node, before and/or after
// the node's children. See Apply for the interpretation of the return
// value.
//
// The node is given by the node pointer's address addr which makes it
// possible for an ApplyFunc to rewrite (replace) the node. The node
// is referred to from its parent which contains a field with the given
// name. If the field is a list, index identifies the node's position in
// that list; index is < 0 otherwise. Or roughly speaking:
//
//   addr == &parent.name         if index < 0
//   addr == &parent.name[index]  if index >= 0
//
// Exception: If the parent is an *ast.Package, and Apply is iterating
// through the Files map, name is the filename, and index is incremented
// by 1 with each file, starting at 0. Assigning to *addr will change the
// corresponding Files map entry but only upon return from pre and post.
type ApplyFunc func(parent Node, name string, index int, addr interface{}) bool

// Apply traverses a syntax tree recursively, starting with the node
// identified by parent, name, index, and addr. See Apply for the meaning
// of these arguments.
//
// If pre is not nil, it is called for each node before its children
// are traversed (pre-order). If the result of calling pre is false,
// no children are traversed, and post is not called for the node.
//
// If post is not nil, it is called for each node after its children
// were traversed (post-order). If the result of calling post is false,
// traversal is terminated and Apply returns immediately.
//
// Only fields that refer to AST nodes are considered children;
// specifically, token.Pos, Scopes, Objects, and fields of basic types
// (strings, etc.) are ignored. Children are traversed in the order in
// which they appear in the respective node's struct definition.
func Apply(parent Node, name string, index int, addr interface{}, pre, post ApplyFunc) {

@mdempsky points out that alternatively, index could simply be monotonically increasing. The index of list element would be the provided index minus the index of the list. A utility function could provide the name of a field for a given index if necessary. That function could be reflection-based since it's probably not speed-critical.

We probably still want the field name though, so that you can locate the subtree easily in cases like SliceExpr without having to do four separate comparisons---and arguably more importantly, remembering to do four separate comparisons.  I have found several bugs caused by assuming that a given node was the "correct" subtree of the parent node, when in fact it was another one.  Passing the field name will help to make it obvious that this is something you need to think about.

I think the easier solution is to still reserve indices for nil fields.  So for SliceExpr you'd always have 0==X, 1==Low, 2 == High, 3==Max.

You can't use the same number to indicate both a field index and a slice index.

I played with this a bit and I have a prototype that could work. Observations: Passing an address to a field is problematic: Many fields that we are interested in are (ast.)Expr, Stmt, and Decl fields, which are interfaces. To "unpack" them we have to type-switch on the address, then deref the address and then type-switch again on the contents; thus requiring two type switches. Here's a better approach: If we have the parent, field name (or field index), plus slice index if needed, we have all that is necessary because we can use reflect to set a field with this information. Changing/rewriting fields is (probably) much less common then reading (traversing) the tree, thus the more costly set field operation is ok. In turn, the API can be closer to Walk and thus easier to use:
// An ApplyFunc is invoked by Apply for each node n, before and/or after
// the node's children. See Apply for the interpretation of the return
// value.
//
// The parent, name, and index field are used to identify the parent
// node's field containing n. If the field is a list, index identifies
// the node's position in that list; index is < 0 otherwise. Roughly:
//
//   n == parent.name         if index < 0
//   n == parent.name[index]  if index >= 0
//
// This information can be used to update/rewrite a node with SetField.
//
// Exception: If the parent is a *Package, and Apply is iterating
// through the Files map, name is the filename, and index is -1.
type ApplyFunc func(parent Node, name string, index int, n Node) bool

// Apply traverses a syntax tree recursively, starting with the node
// identified by parent, name, index, and n. See Apply for the meaning
// of these arguments.
//
// If pre is not nil, it is called for each node before its children
// are traversed (pre-order). If the result of calling pre is false,
// no children are traversed, and post is not called for the node.
//
// If post is not nil, it is called for each node after its children
// were traversed (post-order). If the result of calling post is false,
// traversal is terminated and Apply returns immediately.
//
// Only fields that refer to AST nodes are considered children.
// Children are traversed in the order in which they appear in the
// respective node's struct definition.
func Apply(parent Node, name string, index int, n Node, pre, post ApplyFunc)

// SetField sets the named field in the parent node to n. If the field
// is a slice, index is the slice index. The named field must exist in
// the parent, n must be assignable to that field, and the field must be
// indexable if index >= 0. Roughly:
//
//   parent.name        = n  if index < 0
//   parent.name[index] = n  if index >= 0
//
// The parent node may be a pointer to the struct containing the named
// field, or it may be the struct itself.
//
// Exception: If the parent is a Package, n must be a *File and name is
// interpreted as the filename in the Package.Files map.
func SetField(parent Node, name string, index int, n Node)

Open questions:

Should Apply require parent, name, and index (which often will be nil, "", -1) so that even the top-level node can be rewritten, or should that require a special case and in return a simpler Apply that just requires, the starting node n, pre, and post?

Should field addressing happen via a name (string), which is nice to read but more expensive than say just the field index as provided by reflect)?

On 14 September 2016 at 23:52, Robert Griesemer<EMAIL_ADDRESS>wrote:

If we have the parent, field name (or field index), plus slice index if
needed, we have all that is necessary because we can use reflect to set a
field with this information. Changing/rewriting fields is (probably) much
less common then reading (traversing) the tree, thus the more costly set
field operation is ok. In turn, the API can be closer to Walk and thus
easier to use:

Sounds good.  It's interesting that Apply is now a primitive interface for
traversing AST tree nodes and edges, whether or not you wish to update the
tree.  In other words, you could define the existing ast.Walk and
ast.Inspect functions in terms of Apply.  That might be a good exercise to
validate the API.

// Apply traverses a syntax tree recursively, starting with the node
// identified by parent, name, index, and n. See Apply for the meaning
typo: "See ApplyFunc"

Should Apply require parent, name, and index (which often will be nil,

"", -1) so that even the top-level node can be rewritten, or should that
require a special case and in return a simpler Apply that just requires,
the starting node n, pre, and post?

I don't have a good sense of how important this case will be.  You could
provide a wrapper for the root case with the three zero arguments.

Should field addressing happen via a name (string), which is nice to
read but more expensive than say just the field index as provided by
reflect)?

I see three reasons to prefer field names over field indices:
(1) Client code that uses strings will be easier to read.  Unnamed integer
constants are inscrutable and error-prone.
(2) Strings are invaluable during debugging.  You can print them out to
document the descent, something that is hard to do with the existing (read
only) traversal API.
(3) The AST is bound to change, if only slightly, in future versions of Go,
and we should not assume that all new fields will be added at the end of
their struct, thus the field numbering may not be durable in the long term.

@alandonovan ast.Walk was written w/o a very good understanding of use cases; in fact, I introduced ast.Inspect later because it was much easier to use in many situations. We cannot remove these functions but we could mark them as deprecated. ast.Apply should cover both use cases nicely.

I didn't mean you should remove or deprecate them, only that you should go through the exercise of implementing the old functions in terms of the new, since it might be revealing.

@alandonovan I understand. I think it can be done but it will be difficult to guarantee 100% semantic equivalence w/o manually checking each case, or an extensive test suite testing each node type and exit scenario. For one, Walk doesn't invoke the visitor if a node is nil, while Apply probably should (and my prototype currently does) so that the pre/post closures have a chance to rewrite/set the node (that feature will make it possible to implement Walk, while Walk could not be used to implement Apply since it misses nodes).

You can't use the same number to indicate both a field index and a slice index.

I think we're talking past each other.  Examples of how I use a single index to identify child nodes:

To find the i'th child Node of a CallExpr: i==0 means Fun, and otherwise you want Args[i-1].
To find the i'th child of an AssignStmt, i<len(Lhs) means Lhs[i], and otherwise you want Rhs[i-len(Lhs)].

That said, I'm not opposed to field names.

On 15 September 2016 at 12:48, Matthew Dempsky<EMAIL_ADDRESS>wrote:

You can't use the same number to indicate both a field index and a slice
index.
I think we're talking past each other. Examples of how I use a single
index to identify child nodes:

To find the i'th child Node of a CallExpr: i==0 means Fun, and
otherwise you want Args[i-1].
To find the i'th child of an AssignStmt, i<len(Lhs) means Lhs[i],
and otherwise you want Rhs[i-len(Lhs)].

That said, I'm not opposed to field names.

Ah, so you would flatten out the scalar fields and the slices into one
sequence.  Makes sense, though the bookkeeping seems onerous.

For a concrete implementation, see:
https://github.com/griesemer/dotGo2016/commit/f0f16c266e81095ef09f9af10a03df149c8f3488

I recently started on a tool for vim-go and wanted to easily rewrite AST as well. I've seen this issue and really looking forward what we get out from this. Meanwhile, I've copied ast.Walk() and implemented a straight forward rewriter by changing the signature of the walk function we pass. Here is a working repo: https://github.com/fatih/astrewrite I'm going to use it for my next project. It might be not perfect or might have errors (which are unknown to me know), but I thought I'll share to show a different API usage.
Also I think, we can introduce a ast.Rewrite function without breaking the current API or add it to the golang.org/x/tools/go/ast/astutil.

@fatih Any reason the version I mentioned above ( griesemer/dotGo2016@f0f16c2 ) wouldn't have worked for you?

@griesemer at first the API was a little bit confusing for me. So I didn't tried to investigate in depth. But I've tried to give it a look now that you asked for it.
There are more steps involved changing a node with Apply().  Using the example src below
src := `package main

type Foo struct{}`

fset := token.NewFileSet()
file, _ := parser.ParseFile(fset, "foo.go", src, parser.ParseComments)

I've tried to rename the type name from Foo to Bar. here is how we can do it with Apply() and with Walk() from the astrewrite package"
1. Apply:
applyFunc := func(parent ast.Node, name string, index int, n ast.Node) bool {
	spec, ok := parent.(*ast.GenDecl)
	if !ok {
		return true
	}

	x, ok := n.(*ast.TypeSpec)
	if !ok {
		return true
	}

	x.Name.Name = "Bar"
	spec.Specs[index] = x
	return true
}

rewritten := ast.Apply(file, applyFunc, nil)

2. Walk() from astrewrite:
rewriteFunc := func(n ast.Node) (ast.Node, bool) {
	x, ok := n.(*ast.TypeSpec)
	if !ok {
		return n, true
	}

	x.Name.Name = "Bar"
	return x, true
}

rewritten := astrewrite.Walk(file, rewriteFunc)

With Apply() I have to know the parent nodes type as well, so I can replace the item via the index. However in Walk() from astrewrite I don't need to know the parent node at all. I only change the node I'm interested in and then return it back.
--
Note that my current use case is very simple. There might be other use cases that Apply() covers that I'm not aware of. I'm just trying to give my feedback as a regular Go user that occasionally uses the go/ast package for writing or improving existing tools.

@fatih Thanks for looking into this. A few observations:

When you use Apply, you don't need to type-assert on the parent - you can simply use the new SetField function (also documented in the API): ast.SetField(parent, name, index, <newnode>). With that, both versions will be approximately the same amount of code to write.
In this specific case where you simply change a string, there's no need to use a complicated rewrite in the first place: You could just walk the tree and change that string directly.
Your implementation of astrewrite is easier to use in simple cases, but it is almost always more costly: The implementation of astrewrite always needs to write back the result returned by rewriteFunc, and because it's always an ast.Node there's always a type assertion needed for the assignment to work (in the astrewrite implementation) - that's relatively costly. You have to always do this for every single node even though most nodes are not rewritten. On the other hand, with ast.Apply, the ast.SetField operation is relatively expensive but you only pay when you actually rewrite something.
Finally, with ast.Apply you have the choice to do the rewriting in pre- or post-order when traversing the tree, which is often important to be able to control - this is not possibly with astrewrite.

To summarize: If you only want to change the name of things, you may not need a rewrite at all. Secondly, while astrewrite may be fine for your (more general) use case, it's probably not sufficiently powerful as a general rewrite mechanism, and likely quite a bit slower than the suggested ast.Apply. Hope that helps.
Anyway, thanks again for investigating!

@griesemer Thanks a lot for the detailed explanation.  I've didn't know that I could just use SetField, missed that totally. My example was simple by purpose to show the API. You're right that it's costly and I totally agree, however the API is also simple. It feels intuitive. But this library is not the place to make things feel good in expense of performance :)
I'll try to use Apply() instead of astrewrite for my next project. If there is anything I think it can be improved I'll comment here. Thanks again 👍

@griesemer it'd be handy if that code was go-gettable, so it is easier to use and evolve.
Since you wrote it, perhaps you want to put it up on github under your name? Or I could put it up somewhere for you; let me know.
I've pulled it out of package ast for you: https://gist.github.com/josharian/ff74a8451d7d4e7f062d7b9b04c87eac. Not tested yet, but it compiles, and the only modification was adding ast. in a bunch of places. :)

@josharian Since it's basically there (but for tests), I made this a 1.9 item. It should just be in the ast package. We can put it in once the tree opens (which is hopefully soon).

I'm struggling with deleting elements of an AST.
Sample:
func f() {
  a1()
  a2()
  b()
}

Task: eliminate all statements in f that are calls to functions starting with a.
I did the obvious thing and wrote a Delete helper, analogous to SetField (not thoroughly tested yet):
// Delete deletes or sets to nil the named field from the parent node.
// Delete performs the following assignment:
//
//   if index < 0 {
//     parent.name = nil
//   } else {
//     copy(parent.name[index:], parent.name[index+1:])
//     parent.name[len(parent.name)-1] = nil
//     parent.name = parent.name[:len(parent.name)-1]
//   }
//
// The parent node may be a pointer to the struct containing the named
// field, or it may be the struct itself.
//
// TODO: do parents of Package type need special handling?
func Delete(parent ast.Node, name string, index int) {
	v := reflect.Indirect(reflect.ValueOf(parent)).FieldByName(name)
	if index < 0 {
		v.Set(reflect.Zero(v.Type()))
		return
	}
	last := v.Len()
	reflect.Copy(v.Slice(index, last), v.Slice(index+1, last))
	v.Index(last - 1).Set(reflect.Zero(v.Type().Elem()))
	v.SetLen(last - 1)
}

But when you delete an element from a slice, it disrupts the walk that Apply does. In the sample above, the obvious implementation of pre deletes a1() but misses a2(), because after the deletion of a1(), Apply's index into the BlockStmt is off by one.
One option is give ApplyFunc access to Apply's internal state, perhaps something like this:
type ApplyLocation struct {
  name string // export?
  index int // export?
  parent Node // export?
  a *application
}

type ApplyFunc func(loc ApplyLocation, n Node) bool

func (x ApplyLocation) Set(n ast.Node)

func (x ApplyLocation) Delete()  // adjusts application's internal state as necessary

@josharian Another option might be to buffer the deletes and apply them after the walk - though that may not be desirable in certain situations where we want to the delete to take effect during the walk.
The API will need to describe what guarantees it is going to provide (we have similar problems with deletes of map entries during map iteration).
I've also been thinking that perhaps the description or "cursor" of the entry in the parent (name, index) may have to be opaque so we have more freedom in the implementation.
Thanks for pointing this out. I think this needs a good solution before we should add Apply to the ast package. (Though I don't have time to focus on this at the moment).

I had a somewhat similar problem in a tree transducer I was toying around with recently. I had my equivalent of the apply function have to return a special flag saying "delete the node I was just given" if it wanted to remove instead of alter the node. I don't know how well that would work in this situation, but it worked for me.

I prototyped the "use an opaque cursor for everything" approach. I'll keep using it for my work and see whether further API issues arise. Input welcome.
Here's the godoc output for that gist:
PACKAGE DOCUMENTATION

package apply
    import "."

FUNCTIONS

func Apply(root ast.Node, pre, post ApplyFunc) ast.Node
    Apply traverses a syntax tree recursively, starting with root, and
    calling pre and post for each node as described below. The result is the
    (possibly modified) syntax tree.

    If pre is not nil, it is called for each node before its children are
    traversed (pre-order). If the result of calling pre is false, no
    children are traversed, and post is not called for that node.

    If post is not nil, it is called for each node after its children were
    traversed (post-order). If the result of calling post is false,
    traversal is terminated and Apply returns immediately.

    Only fields that refer to AST nodes are considered children. Children
    are traversed in the order in which they appear in the respective node's
    struct definition.

TYPES

type ApplyCursor struct {
    // contains filtered or unexported fields
}
    An ApplyCursor describes a node encountered during Apply. Information
    about the node and its parent is available via the Node, Parent, Name,
    and Index methods.

    Roughly speaking, the following invariants hold:

	Parent().Name()          == Node()  if !HasIndex()
	Parent().Name()[Index()] == Node()  if HasIndex()

    The methods Replace, Delete, InsertBefore, and InsertAfter can be used
    to change the AST without disrupting Apply.

func (c ApplyCursor) Delete()
    Delete deletes the current Node from its containing slice. If the
    current Node is not part of a slice, Delete panics.

func (c ApplyCursor) HasIndex() bool
    HasIndex reports whether the current Node is part of a slice of Nodes.

func (c ApplyCursor) Index() int
    Index reports the index of the current Node in the slice of Nodes that
    contains it. Index panics if the current Node is not part of a slice.

func (c ApplyCursor) InsertAfter(n ast.Node)
    InsertAfter inserts n after the current Node in its containing slice. If
    the current Node is not part of a slice, InsertAfter panics. Apply will
    walk n.

func (c ApplyCursor) InsertBefore(n ast.Node)
    InsertBefore inserts n before the current Node in its containing slice.
    If the current Node is not part of a slice, InsertBefore panics. Apply
    will not walk n.

func (c ApplyCursor) IsFile() bool
    IsFile reports whether the current Node is a *File that is part of a
    *Package map of *Files.

func (c ApplyCursor) Name() string
    Name returns the name of the parent Node field that contains the current
    Node. If the parent is a Package and the current Node is a File, it
    returns the filename for the current Node.

func (c ApplyCursor) Node() ast.Node
    Node returns the current Node.

func (c ApplyCursor) Parent() ast.Node
    Parent returns the parent of the current Node.

func (c ApplyCursor) Replace(n ast.Node)
    Replace replaces the current Node with n. The replacement node is not
    walked by Apply.

type ApplyFunc func(cursor ApplyCursor) bool
    An ApplyFunc is invoked by Apply for each node n, even if n is nil,
    before and/or after the node's children.

    The return value of ApplyFunc controls the syntax tree traversal. See
    Apply for details.

@josharian I like your API but the one thing that concerns me is that an ApplyCursor needs to be set up for each node even if it's not needed (which I suspect is 99% of the time because most often an Apply is looking for one specific node only). I wonder if this approach can be made slightly more light-weight w/o losing too much of it's power.

Indeed. Although maybe if we documented that the lifetime of an ApplyCursor is a single call to pre/post, then we could allocate a single ApplyCursor and reuse it. The non-allocation part of the setup of the ApplyCursor should be pretty cheap, I think.
One other thing I learned while using the API above is that frequently I don't want to walk the node inserted by InsertAfter. We might want to change the behavior to non-walk (since the user can manually walk it if desired) or add a bool parameter to control the behavior.

@josharian Yes, if you pass in the ApplyCursor info down it's like passing the apply parameters via the ApplyCursor struct, and it's only allocated once. I like that. Care making this an official CL as solution for this issue (assign to me for review)?

Will do; expect it to take a few days.

Thanks - no rush.

(still working on this, not forgotten)

A quick question. What are valid ast.Node types that I can use to insert before an ast.IfStmt?
A vardecl would indeed not make sense. But I have some trouble inserting an assignmentStatment.  Would that be of type interface ast.Stmt? How would that wrap the ast.AssignStmt type
func vardecl(name string, typ string) ast.Node {
	return &ast.GenDecl{
		Tok: token.VAR,
		Specs: []ast.Spec{
			&ast.ValueSpec{
				Names: []*ast.Ident{ast.NewIdent(name)},
				Type:  ast.NewIdent(typ),
			},
		},
	}
}

func assignStatment() ast.Stmt {
	/*
		return  ast.AssignStmt{
			Lhs: []ast.Expr{&ast.ExprStmt{ ast.Ident{ast.NewIdent(y)}}},
			Rhs: []ast.Expr{ast.BasicLit{
				Kind: token.STRING,
				Value: "4"}},
		}
*/
	return nil
}

	var pre = func(cursor astExtra.ApplyCursor) bool {
		n := cursor.Node()
		switch n.(type) {
		case *ast.IfStmt:
			cursor.InsertBefore(vardecl("before", "string"))
                        //cursor.InsertBefore(assignStatement())
		}
		return true
	}

We don't use the issue tracker for questions; in the future please ask one the mailing lists or chat boards.

An AssignmentStmt is a statement and can be used where a statement would go.
A variable declaration can go into a DeclStmt which can also go where a statement would go.

The go/ast documentation groups the nodes in Decl, Expr, and Stmt, and the grouping is pretty clearly telling you which node fits where. It's also trivial to find out empirically: if you can assign a node x to an ast.Stmt, then x is a statement, etc.

@griesemer sorry. I deleted the question. will try to find help on  the boards

Is there a reason why the proposed ast.Apply returns a (possibly) modified ast.Node? This seems like a departure from ast.Walk, and I don't understand why it is necessary to return a Node instead of modifying it in place.

@smasher164 Because ast.Apply may change the type of the node which cannot be done by modifying a node in place. For instance, an ast.Apply working on an constant expression tree may replace that tree with a single node which is the constant result.


[9.x] Add SQL Parentheses for whereRaw in order to use SoftDelete safely

refs: #2384

So the idea of your problem with operator precedence ("or" vs "and") is to always wrap the where in parenthesis?
Might be a stretch, but maybe people rely on this. You're effectively changing the literal raw query the user provided, which does not sound like a good idea.

I can see your problem though, it's absolutely clear that there an existing where condition applied with interferes with your expectation of operator precedence. But OTOH, it has to be clear that there are additional where conditions, otherwise the trait wouldn't make sense.
Personally, if others agree there's something to fix, it should be done only in this context. But I think the proper "fix" is to maybe improve the docs.
IMHO 😄

IMO, people don't always read document. So if possible, I want enable people to use SoftDelete safely if people don't read document.
But OTOH, I think I don't wanna make destructive change. If it affects another feature, I'll close this PR and I'll improve the docs.

I dunno if I feel comfortable modifying the raw query.

ok, I'll improve the docs.


Let's polish this up for a release?

Let's polish this up, move it up to latest brave and to zipkin2 reporter. It hasn't been released before, so seems a good time.

https://github.com/openzipkin/brave-karaf/pull/5

ok we are just about ready.. I think just circleci needs credentials re-encrypted
tried release-0.1.0 and it failed due to this

attempting a release based on dubbo's guide https://dubbo.incubator.apache.org/en-us/blog/prepare-an-apache-release.html

<EMAIL_ADDRESS>@reta question for you.. I released like this.
``bash
./mvnw release:prepare -Prelease -Darguments="-Dmaven.test.skip=true" -DautoVersionSubmodules=true -Dusername=adriancole -DpushChanges=false
./mvnw release:perform -Prelease -pl -:brave-itests -Darguments="-Dmaven.test.skip=true" -DautoVersionSubmodules=true -Dusername=adriancole

I noticed that eventhough I put a skip in, itests still uploaded. Maybe we need to add a skip to the pom directly?

[INFO] Uploading to apache.releases.https: https://repository.apache.org/service/local/staging/deploy/maven2/org/apache/zipkin/brave/karaf/brave-itests/0.1.2/brave-itests-0.1.2.jar.asc

Good question, @adriancole , I think you need to skip inside the pom.xml direcly for brave-itests, as you suggested:
<plugin>
    <groupId>org.apache.maven.plugins</groupId>
    <artifactId>maven-deploy-plugin</artifactId>
    <configuration>
        <skip>true</skip>
    </configuration>
  </plugin>

Could you please try it out? As a side note, many projects publish all artifacts, seems to be easier :)

heh cool. I will work this in tomorrow morning
On Sat, Feb 9, 2019, 9:54 PM Andriy Redko<EMAIL_ADDRESS>wrote:

Good question, @adriancole https://github.com/adriancole , I think you
need to skip inside the pom.xml direcly for brave-itests, as you
suggested:

org.apache.maven.plugins
maven-deploy-plugin

true

Could you please try it out? As a side note, many projects publish all
artifacts, seems to be easier :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/apache/incubator-zipkin-brave-karaf/issues/4#issuecomment-462046479,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAD616FLkE3TiIe_kEQDMzxpijq-LeTUks5vLtMogaJpZM4SRwvP
.

I left an old "release" profile in the build, and used that instead of the apache one. That's why there was no asc file @abesto
Here's the commands I used to release this time:
$ ./mvnw release:prepare -Papache-release -Darguments="-Dmaven.test.skip=true" -DautoVersionSubmodules=true -Dusername=adriancole -DpushChanges=false
$ ./mvnw release:perform -Papache-release -Darguments="-Dmaven.test.skip=true" -DautoVersionSubmodules=true -Dusername=adriancole


CLIENT_GATEWAY_NETWORK due to CORS error when using Fiddler

General information

SDK version: 3.76.0
Environment: Production
Browser and OS: Chrome + Fiddler on Windows

Issue description
Developers often have tools such as Fiddler open for debugging. For the most part other websites will continue to work as normal, but it seems that CORS errors can occur when trying to make calls to graphql endpoints on payments.braintree-api.com and api.braintreegateway.com if Fiddler is open.
I do want to make it clear that I understand development environments are going to have side-effects on everyday web browsing, but clearly people make purchases on their work computers with developer tools open. Other developers are my customers and this is very hard to diagnose if you can't see a dev tool trace.
This issue does not occur on the hosted fields sandbox demo, but is easily reproduced in production by opening Fiddler and trying to create a client. I randomly picked Casper from your featured merchants to demonstrate but these are the same errors I got on my own website:

 {
   code: "CLIENT_GATEWAY_NETWORK"
   details:
   originalError: "error"
   message: "Cannot contact the gateway at this time."
   name: "BraintreeError"
   type: "NETWORK"
}

This error is frustratingly not handled by most companies, leaving a 'dead' unclickable credit card form with no feedback.
I'm not expecting a magic solution to this issue, but opened it in the hope of exploring some of the following:

Make sure anyone experiencing a CLIENT_GATEWAY_NETWORK error understands that simply having Fiddler open can be a cause!

Suggest that if hosted fields fails to initialize because of this error that maybe a subsequent click on the text box should show a default inline error message or popup suggesting reload / contact us.

Suggest a different error message such as CLIENT_CORS_ERROR that could be raised if CORS could be determined to be the underlying cause.

Unfortunately this may not be possible. I tried putting some breakpoints in client.js but the error itself never surfaced via the XmlHttpRequest call (in the ReadyStateChanged handler). I even manually added an additional onerror handler and couldn't see it. The error details do get printed to the console, but as shown above originalError == 'error' and nothing useful to distinguish the reason.

Point out that the http status code returned from the CORS problem is 0 but client.js converts this to a 500 error with status || 500 when it isn't really a true 500 error.

Confirm that there's no other way your server can be configured to avoid this. I assume it's configured very deliberately for security reasons, but I never really understand CORS beyond the minimum I need to know!

See if anyone has recommendations for how to configure Fiddler to avoid this. I've found some possible solutions but they involve modifying every request with a filter.

Recommend all developers test for this, hide the hosted fields and show an error message - this is what I'm now doing:

I'm talking to our networking team about this to see if there's anything specific to our setup that would cause this.

@crookedneighbor sorry! It looks like I did in fact have some custom rule in Fiddler that was affecting this - that would be rare for anyone else to have.

@simeyla it's all good! We'd rather folks come to us with issues than stay quiet 😄

Hi, I am investigating a sudden increase on the number of this error occurrences in our app. It seems so far that nothing on our end would cause such increasing. It seems to me at this point that this is most likely related to customer network issues then something else. Sorry, but just to clarify, In which cases this error is commonly noticed? If here is not the best place to ask, please point me the correct place.
Thanks.


No support for areas

Would it be possible to add support for areas? The following route:
routes.MapRoute(
	name: "areaRoute",
	template: "api/{area}/{controller}/{id?}"
);

generates the following URL: /api/{area}/controllerName. And this can also generate conflicting actions if there are controllers with the same name in different areas.

@bpiec  can you upgrade and try one of the following versions?

ASP.NET Core 2.2:   <IP_ADDRESS>
ASP.NET Core 3. :    <IP_ADDRESS>

Did you commit the code too?

Just did.

You're awesome! Working like a charm.
I have one question however. Now it groups operations by the controller name. But if you have controllers with the same name in different areas it joins them into the same group (this is called tags in OpenAPI). Is it possible to change that and group by area name (or even Area.Controller)? Where's the code doing this? I thought ApiDescriptionActionData is responsible for that but I cannot see any setter usage for GroupName…

Great news @bpiec , I am closing this issue as resolved.
Your suggestion is very interesting, I am going to investigate it in the near future.

For any future readers – this is how one can easily group operations using areas names:
services.AddSwaggerGen(options =>
{
	// …

	options.TagActionsBy(selector =>
	{
		selector.ActionDescriptor.RouteValues.TryGetValue("area", out var area);
		var controller = selector.ActionDescriptor.RouteValues["controller"];
		return new[] {area != null ? $"{area}.{controller}" : controller};
	});

	// …
});


Updated specification for JSON requests to daemon

Merge new documentation for JSON requests.

New specifications as follows:
{
  "repo_url": "https://the.repo",
  "repo_branch": null || "thebranch",

  // ONE of the following
  "deploy_config": null,   // use "deploy_config_path" (must not be null)
  "deploy_config": { /* JSON of deployment config */ },
  "deploy_config": "{ JSON string of deployment config }",

  // ONE of the following
  "service_config": null,   // use "service_config_path" (must not be null)
  "service_config": { /* JSON of service config */ },
  "service_config": "{ JSON string of service config }",

  // ONE of the following
  "deploy_config_path": null,   // use "deploy_config" (must not be null)
  "deploy_config_path": "path/to/config",

  // ONE of the following
  "service_config_path": null,   // use "service_config" (must not be null)
  "service_config_path": "path/to/config",
}

Changed documentation to what Robert recommended.

cc @Nathan-Hausman for review


carla and tensorlayer import conflict: syntax error

Python3 can import carla, tensorflow, tensorlayer individually, but cannot import all of them together. Is there a variable naming conflict?
Version:
Carla: 0.9.5
tensorflow: 1.12.0
tensorlayer: 1.11.0
python: 3.5.2
Python 3.5.2 (default, Nov 12 2018, 13:43:14)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import carla
>>> import tensorflow as tf
>>> import tensorlayer as tl
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.5/dist-packages/tensorlayer/__init__.py", line 47, in <module>
    from tensorlayer import initializers
  File "/usr/local/lib/python3.5/dist-packages/tensorlayer/initializers.py", line 7, in <module>
    from tensorlayer.layers.core import LayersConfig
  File "/usr/local/lib/python3.5/dist-packages/tensorlayer/layers/__init__.py", line 31, in <module>
    from .recurrent import *
  File "/usr/local/lib/python3.5/dist-packages/tensorlayer/layers/recurrent.py", line 8, in <module>
    from tensorflow.contrib.rnn import stack_bidirectional_dynamic_rnn
  File "/usr/local/lib/python3.5/dist-packages/tensorflow/contrib/__init__.py", line 57, in <module>
    from tensorflow.contrib import image
  File "/usr/local/lib/python3.5/dist-packages/tensorflow/contrib/image/__init__.py", line 70, in <module>
    from tensorflow.contrib.image.python.ops.single_image_random_dot_stereograms import single_image_random_dot_stereograms
  File "/usr/local/lib/python3.5/dist-packages/tensorflow/contrib/image/python/ops/single_image_random_dot_stereograms.py", line 27, in <module>
    "_single_image_random_dot_stereograms.so"))
  File "/usr/local/lib/python3.5/dist-packages/tensorflow/contrib/util/loader.py", line 56, in load_op_library
    ret = load_library.load_op_library(path)
  File "/usr/local/lib/python3.5/dist-packages/tensorflow/python/framework/load_library.py", line 77, in load_op_library
    exec(wrappers, module.__dict__)
  File "<string>", line 28
    def single_image_random_dot_stereograms(depth_values, hidden_surface_removal=True, convergence_dots_size=8, dots_per_inch=72, eye_separation=, mu=, normalize=True, normalize_max=, normalize_min=, border_level=, number_colors=256, output_image_shape=[1024, 768, 1], output_data_window=[1022, 757], name=None):

                                                                                ^
SyntaxError: invalid syntax

I got a temporary work around by import carla in the end.

opened ticket on tensorflow https://github.com/tensorflow/tensorflow/issues/34828 project

This should be solved now in newest versions.


Allow reusing the same context across multiple Encoders and Decoders

From profiling, I have found that creating a zstd::stream::Encoder and using it to encode a small amount of bytes (less than 100) takes over 200 microseconds per encode on my machine. The vast majority of this time is related to the creation of new zstd contexts, so this PR adds a simple method on Encoder and Decoder that allows one to supply an external context from zstd_safe instead. With these changes, performance is improved - the previously-mentioned benchmark takes less than 2 microseconds per encode.
This PR also makes some minor changes to the WASM shim, as compilation appears to be broken for it.

would love to see this!

bump @gyscos for visibility

Thanks for the PR!
A few questions:

Since this is all internal API, why separate MaybeOwned and MaybeOwnedInner types?
Why store a * mut () + PhantomData rather than directly a &'a mut T?

Since this is all internal API, why separate MaybeOwned and MaybeOwnedInner types?

My instinct was to separate them for readability. However, since it's an internal API, they don't necessarily have to be separate. I will go ahead and change this.

Why store a * mut () + PhantomData rather than directly a &'a mut T?

The goal for doing this was to ensure that Encoder<'a> and Decoder<'a> were covariant over 'a. If &'a mut T was used, the function with_context<'b: 'a>(reader: R, context: &'a mut zstd_safe::DCtx<'b>) would not compile - it would be impossible to convert a &'a mut zstd_safe::DCtx<'b> to a &'a mut zstd_safe::DCtx<'a>.
However! Looking at this again, MaybeOwned<T> actually appears to be unsound (see this playground for example). As such, I am going to refactor with_context to just be fn with_context(reader: R, context: &'a mut zstd_safe::DCtx<'static>). I am unable to find a safe way to support having a borrowed DCtx<'a> for an arbitrary 'a - my only idea is to add another lifetime parameter to Encoder and Decoder, but that breaking change doesn't seem worth it. If someone knows of a better approach, please let me know.

I don't seem to see these changes anymore, were they already merged through another PR?

Yes, the WASM shim now works properly on main.

Taking another look at this PR, I feel like the cleanest solution might be to create an entirely separate Encoder and Decoder class for borrowed contexts, rather than trying to support both owned/borrowed contexts with one struct. But I don't have the time do any more API design than this. Please let me know if you have any other suggestions for pushing the PR forward. Thanks!

Ping @gyscos

I found a workaround that reuses the dictionary: https://gist.github.com/esemeniuc/ad889c617cc4297043fc344136be563f
I'd guess its probably 95% as fast

Bump @gyscos

Thanks for the work! Let's start with that, we'll see later if/how we could allow both referencing a dict and a context.

CI failures looks weird - maybe you need to rebase?

Ended up testing and merging the PR locally. Thanks for the work!


Function documentation correction

Removed state param in docs for hashFull.

Good catch, I forgot about the ditto.


Report time support

Add an option to calculate and report the time support within the query of the matching hashes i.e. the earliest and latest times of found matching hashes. (This also gives the time support within the reference when added to the skew time). Useful for organizing multiple matching excerpts in a single query, even if they come from arbitrary places within ref items.

Closed via https://github.com/dpwe/audfprint/commit/c3aeaafe7c185dedcf041478ead176de714f5aa9


Quietly restore a session

This PR makes it possible to quietly restore a session.
require("auto-session").setup({
  restore_quietly = true,
})

This should remove the second confirmation of rmagatti/session-lens#17.

@tiagovla thanks for addressing the changes! Would you mind adding something to the README about this option? If not that's alright too, I'll add some stuff later today or tomorrow!


Installation OK but error opening admin Cannot Get /

Installation went OK
But when I want to open admin server IP (localhost / <IP_ADDRESS>)
I only get Cannot GET / And few errors on console

Using firefox on Windows 10
Here's my terminal:

Need help thanks

Nvm, I should open http://localhost/admin instead of just http://localhost


[Bug - next-pwa]: 'Worker': URL object could not be cloned.

Provide environment information
DOMException: Failed to execute 'postMessage' on 'Worker': URL object could not be cloned.
I often get this error in the console. And I don’t know how to fix it, please help me
Link to reproduction - Issues with a link to complete (but minimal) reproduction code help us address them faster

To reproduce
Most likely this happened after I enabled the option cacheOnFrontEndNav
Describe the bug
Sometimes an error appears in the console:
Uncaught (in promise) DOMException: Failed to execute 'postMessage' on 'Worker': URL object could not be cloned.
at t (https://domain/_next/static/chunks/main-3bc898d879bed56f.js?WB_REVISION=3bc898d879bed56f:1:116782)
at History. (https://domain/_next/static/chunks/main-3bc898d879bed56f.js?WB_REVISION=3bc898d879bed56f:1:116928)
at XB.c. [as pushState] (https://www.googletagmanager.com/gtag/js?id=G-S7VMCNJQPQ:532:249)
at v (https://domain/_next/static/chunks/pages/_app-eeb8b33192225c22.js?WB_REVISION=eeb8b33192225c22:49:128754)
at https://domain/_next/static/chunks/8387.0dc61092ce800a14.js?WB_REVISION=0dc61092ce800a14:1:33306
at uU (https://domain/_next/static/chunks/framework-14a1e7bcaae592ff.js?WB_REVISION=14a1e7bcaae592ff:9:84108)
at oV (https://domain/_next/static/chunks/framework-14a1e7bcaae592ff.js?WB_REVISION=14a1e7bcaae592ff:9:113154)
at https://domain/_next/static/chunks/framework-14a1e7bcaae592ff.js?WB_REVISION=14a1e7bcaae592ff:9:111540
at oU (https://domain/_next/static/chunks/framework-14a1e7bcaae592ff.js?WB_REVISION=14a1e7bcaae592ff:9:111606)
at oP (https://domain/_next/static/chunks/framework-14a1e7bcaae592ff.js?WB_REVISION=14a1e7bcaae592ff:9:96235)
Expected behavior
No errors, or at least for the errors to disappear after disabling this option
Screenshots (if relevant)

Additional information (if relevant)
No response

Oops, forgot this. Fixing this soon!


ICMP Support

Brickner[CodePlex] Add support for ICMP datagrams (over IPv4).
Should looks like this:
Packet packet;
packet.Ethernet.IpV4.Icmp...

Hi. What's about ICMPv6? I can't find it, is it implemented?

Try packet.Ethernet.IpV6.Icmp


[Bug]: Local Void issues

Is there an existing issue for this?

[X] I have searched the existing issues

Description
I've noticed some issues with the local void.

When local void teleports are set to free, you can only teleport if you have enough experience as if the teleport wasn't free. Otherwise it says you don't have enough experience.
If the local void teleports are set to not free, you simply can't teleport, attempting to use the local void does nothing.

To Reproduce

Set local void teleports to free.
Set a distance based cost on waystones.
Notice that not having enough xp for teleport won't let you use local void.
Once you have enough xp, you can teleport and no experience is lost.
Set local void teleports to not free
Notice local void interaction does nothing, always unable to teleport with no error message.

Expected behavior
Local void
Mod Version
v3.1.1 or newer
Minecraft Version
1.20 or newer
Java Version
Java 18+
Fabric Version
No response
Relevant log output
No response
Anything else?
No response

Yikes, surprised no one reported this sooner...


tests: Bluetooth: CAP: Remove dup adv in acceptor bsim test

Remove a duplicated advertising set in the CAP acceptor babblesim test. The advertising set is configured in test_start_adv.

CI failure is unreleated to this change

@cvinayak is this expected to fail?

@cvinayak is this expected to fail?

Hopefully https://github.com/zephyrproject-rtos/zephyr/pull/80733 should help... will give a re-run when thats merged.

@cvinayak is this expected to fail?

Hopefully #80733 should help... will give a re-run when thats merged.

Seems to still be an issue :(

Was merged by https://github.com/zephyrproject-rtos/zephyr/pull/82199


Broken contract for editor model

New contract is blocking some cases with language id
Classic way:
monaco.editor.createModel(source, langueageId);

New way:
const modelRef = await createModelReference(monaco.Uri.file('/tmp/test.js'), source)

How i can determine language id?
Why the contact of createModelReference is different from monaco.editor.createModel?
Also please use uri as non required argument like in normal contract.

createModelReference is an internal api of vscode, the language is detected from the file name and content.
If the detection failed, you can still usemonaco.editor.setModelLanguage on the returned model

Also please use uri as non required argument like in normal contract.

The point of this method is to create a reference to a model via a path. generating a random path here (what monaco.editor.createModel is doing when no uri is provided) has no point. You can still use monaco.editor.createModel is such cases

createModelReference contract is designed by vscode, right?

What do you mean?

Yes, I've just added the source parameter which write the file on the virtual filesystem before calling the internal api


Documenting quadratic-Bezier exact-arc-length function

I'll try to document the function below:
https://github.com/fonttools/fonttools/blob/f1c609aa57fa11ab98f2152275f2c709e06c0680/Lib/fontTools/misc/bezierTools.py#L205-L234
If the quadratic Bezier is defined as $$B(t) = pt1(1-t)^2 + pt2.t(1-t) + pt3.t^3$$ then the arc length is $$\int_0^1 |dB(t)/dt|dt$$ where $$dB(t)/dt  = 2(pt2 - pt1)(1- t) + 2(pt3 - pt2)t$$
Others have solved this integral to a long closed-form solution involving the log function. The algorithm here uses trigonometric functions instead and in my opinion is simpler. The trick is to use a transformation to simplify the integral, which otherwise is hard to work with.
Observe that $dB(t)/dt$ is of linear form in $t$. Which is a property of Bezier curves in general, which is that the derivative of a Bezier curve of degree $n$ itself is a Bezier curve of degree $n-1$. As such, the derivative of a quadratic Bezier is a line segment.
The arc length of a the quadratic Bezier then is the integral of the distance-to-origin over a line segment. This integral is generally hard to work with. However, such an integral can be easy to find in closed form for lines of certain form. For example, and we are going to use this, the line of $x = 1$. All we need to do is to rotate and scale our line segment to align it with the $x=1$ line and do the integration, which now looks like $$\int_{y_0}^{y_1} \sqrt{1+y^2} dy$$
Let's bring in some trigonometric functions in. Let $y = \tan(\theta)$, so $\theta = \text{atan}(y)$. Observe that $\sqrt{1+y^2} = \sec(\theta)$. Plugging those into the integral we get $$\int_{y_0}^{y_1} \sec(\text{atan}(y)) dy$$ Finally! This integral has a closed-form solution in terms of the inverse hyperbolic functions: $$x\sqrt{1+x^2}/2 + \text{asinh}(x)/2$$
And that's exactly what the code does. It doesn't perform any actual rotation of the line segment. It uses vector math to simulate that. (The code calls $y_0,y_1$ x0,x1 instead.

@Pomax @raphlinus


Default interaction profile should be auto-selected based on runtime name

To avoid 2 interaction profiles from fighting each other (eg: Touch controllers for physical controllers and G2 for  hand emulation), add an auto-select option that will use the runtime name to make a best decision.

Actually a better approch is probably to block all other suggested bindings.


Multiple gateways fails to reach brokers.

Openshift version: 3.11
Helm version: 3
Zeebe version: 0.22.1
How to reproduce ?
Deploy helm chart with 3 replicas of gateway and broker. Broker pod results in below error.
2020-05-11 06:09:30.301 [] [atomix-partition-group-membership-service-0] WARN  io.atomix.primitive.partition.impl.DefaultPartitionGroupMembershipService - Failed to locate management group via bootstrap nodes. Please ensure partition groups are configured either locally or remotely and the node is able to reach partition group members
But it works ok when deployed with 1 replica of gateway and 3 replica of broker. No config change.

This log is not an issue. Yes it is confusing it is removed in the newest versions, but it is not a problem. Do you have problems to reach the brokers with more then one gateway ?

Yes, couldnt reach brokers if the gateway is more than one.

Thanks for conforming. We need to have a look maybe we have some questions later.

Can't reproduce this issue with the our current snapshot version.
I run our normal test benchmark with three gateways and it seem to work.

I used the following configuration:
image:
  repository: camunda/zeebe
  tag: SNAPSHOT
  pullPolicy: Always

# ZEEBE CFG
clusterSize: 3
partitionCount: 3
replicationFactor: 3
cpuThreadCount: 4
ioThreadCount: 4
gatewayMetrics: true

gateway:
  replicas: 3
  logLevel: debug

# Uncomment to add custom YAML configuration
# This will overwrite anything at the application.yml location
# zeebeCfg: {}

# JavaOpts:
# DEFAULTS
JavaOpts: >
  -XX:+UseParallelGC
  -XX:MinHeapFreeRatio=5
  -XX:MaxHeapFreeRatio=10
  -XX:MaxRAMPercentage=25.0
  -XX:GCTimeRatio=4
  -XX:AdaptiveSizePolicyWeight=90
  -XX:+PrintFlagsFinal
  -Xmx4g
  -Xms4g
  -XX:+ExitOnOutOfMemoryError
  -XX:+HeapDumpOnOutOfMemoryError
  -XX:HeapDumpPath=/usr/local/zeebe/data
  -XX:ErrorFile=/usr/local/zeebe/data/zeebe_error%p.log

# Environment variables
env:
  # Enable JSON logging for google cloud stackdriver
  - name: ZEEBE_LOG_APPENDER
    value: Stackdriver
  - name: ZEEBE_BROKER_EXECUTION_METRICS_EXPORTER_ENABLED
    value: "true"

# RESOURCES
resources:
  limits:
    cpu: 5
    memory: 12Gi
  requests:
    cpu: 5
    memory: 12Gi

# PVC
pvcAccessMode: ["ReadWriteOnce"]
pvcSize: 128Gi
pvcStorageClassName: ssd

# ELASTIC
elasticsearch:
  enabled: true
  imageTag: 6.8.5

  replicas: 3
  minimumMasterNodes: 2

  volumeClaimTemplate:
    accessModes: [ "ReadWriteOnce" ]
    storageClassName: "ssd"
    resources:
      requests:
        storage: 300Gi

  esJavaOpts: "-Xmx4g -Xms4g"

  resources:
    requests:
      cpu: 3
      memory: 8Gi
    limits:
      cpu: 3
      memory: 8Gi

# KIBANA
kibana:
  enabled: false

# PROMETHEUS
prometheus:
  enabled: false

  servicemonitor:
    enabled: true

What is the reason to use 0.22.1 instead of 0.23.1?

I think 0.22.1 will not work, since the helm-chart uses the new environment variables which are introduced with 0.23.x
Could you please try it with 0.23.x

Thanks @Zelldon . New version with new helm chart works. Just curious, why ES version is 6.8.5  ?

Awesome! Happy to hear that. :slightly_smiling_face:
I will close this issue then.
Regarding to elastic version, the Broker doesn't support 7.x yet, see https://github.com/zeebe-io/zeebe/issues/3445


catch api exceptions when migrating rbs to rogue

What's this PR do?
Adds a catch for DoSomething\Gateway\Exceptions\ApiException like we do in the Rogue module. Hopefully this will catch the 422s we are getting (if there are more after that first one) when testing on Thor. I'm thinking it might be helpful to get everything not causing trouble migrated over so we can see how many are actually causing trouble and try to find a pattern.
Checklist

[ ] Tested on staging.

should we add these into a log?

@angaither like throw the ones that failed this way into their own table so we can keep better track of them?

@katiecrane maybz, or a log file that's probably easier. I think @DFurnes did something like this for a NS script?

@angaither oooo yeah that'd probably be easier and just as good for what we need. I'll dig around


AbstractItem exception on ViewHolder instantiation exception is misleading

Exceptions during ViewHolder construction lead to this line which says:
throw new RuntimeException("You have to provide a ViewHolder with a constructor which takes a view!"); which is not the cause in this situation.
Currently to get the real reason of a problem there's a need to set a breakpoint at this line to check e variable value. It would be good for a faster debugging to change that line.

There could be no need for this anymore in a newer version.
https://github.com/mikepenz/FastAdapter/pull/353

I close this issue as it will be completely fixed in v3 and is no real problem in v2.


Support tags on tracked characters

These would extend a tracked character with a list of strings that can be added to and removed from.
Usable for tracking conditions, etc.
Display them in the combat sticky?

Done


cgp-0130

@reneerrusso can you remove execution date pls

@reneerrusso  And also Governance Proposal Id this is only known when we submit the proposal OnChain


Create Frontend Routes

We will need routes and a document of how to route for our website. We will have pages such as

/login/
/admin/           (admin dashboard)
/admin/createaccount/      (where an admin can create an account for someone else)
/masterteacher/     (master teacher dashboard)
By the end of this sprint, please have the routing for our website. They do not have to lead to real pages yet. You may find the design team's current site map very useful for this task (feel free to leave them comments if you think that it may be done differently).

If you need more information about the features of our website, please reach out to Allen/Anikait or reference the Requirements Document!
closes #5

11/20
Created frontend routes draft
Added base React Router framework to frontend repo


Pfnd3dddiPresent1 2nd parameter

The second parameter to the Pfnd3dddiPresent1 function is unnamed and has no description or documentation.

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: cde12cc4-2c56-1b6d-5dcb-e64a02e5c89b
Version Independent ID: ef0e8d82-5d04-1e85-9085-7b4f2c533991
Content: PFND3DDDI_PRESENT1
Content Source: wdk-ddi-src/content/d3dumddi/nc-d3dumddi-pfnd3dddi_present1.md
Product: windows-hardware
GitHub Login: @windows-driver-content
Microsoft Alias: windowsdriverdev

Thanks, @englishb
@mimisasouvanh can you please get this taken care of? Thanks!

@englishb Hi Ben,
I made the fix. You should be able to see the second parameter in the next day.
Thanks,
Mimi


[improve][cli]Introduce Regex and File Input Parameters for Enhanced Topic Deletion Command

PIP: https://github.com/apache/pulsar/issues/20225
Motivation
The current implementation of the DeleteCmd class in the Pulsar CLI does not provide the flexibility to delete multiple topics at once using a regex or a file containing a list of topics. This can be inconvenient and time-consuming in environments where there are many topics that need to be deleted.
Modifications
To address this, we have added three new parameters to the DeleteCmd class:

-r or regex: This parameter allows the user to specify a regex to match multiple topics for deletion.
--from-file: This parameter allows the user to read a list of topics from a file for deletion.

Additionally, we have written unit tests to ensure the correct behavior of these new features. These tests cover the deletion of a single topic, multiple topics via regex, and multiple topics from a file.
Verifying this change

[ ] Make sure that the change passes the CI checks.

(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

[ ] Dependencies (add or upgrade a dependency)
[ ] The public API
[ ] The schema
[ ] The default values of configurations
[ ] The threading model
[ ] The binary protocol
[ ] The REST endpoints
[ ] The admin CLI options
[ ] The metrics
[ ] Anything that affects deployment

Documentation

[ ] doc
[ ] doc-required
[x] doc-not-needed
[ ] doc-complete

Matching PR in forked repository
PR in forked repository:

Codecov Report

Merging #21664 (f965eda) into master (2a5aa44) will decrease coverage by 0.13%.
Report is 42 commits behind head on master.
The diff coverage is 0.00%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #21664      +/-   ##
============================================
- Coverage     36.73%   36.60%   -0.13%
- Complexity    12195    12241      +46
============================================
  Files          1715     1716       +1
  Lines        131087   131195     +108
  Branches      14317    14335      +18
============================================
- Hits          48155    48025     -130
- Misses        76543    76759     +216
- Partials       6389     6411      +22

Flag
Coverage Δ

inttests
24.02% <0.00%> (-0.17%)
:arrow_down:

systests
24.73% <0.00%> (-0.28%)
:arrow_down:

unittests
31.75% <ø> (+0.07%)
:arrow_up:

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

Files
Coverage Δ

...in/java/org/apache/pulsar/admin/cli/CmdTopics.java
0.00% <0.00%> (ø)

... and 119 files with indirect coverage changes


The "you" is displayed instead of "your" in the camera permission pop-up

Precondition:
The user is disallowed the camera permission
The user sees the "We can’t access you camera" pop-up after trying to verify
Steps to reproduce:

Pay attention to the "We can’t access you camera"  title

Actual result: The "you" is displayed instead of "your" in the camera permission pop-up
Expected result: The "We can’t access your camera" pop-up should be displayed
Checked on QA V0.22.1-1
Reproduced on:
Desktop: Windows 10 // Google Chrome 83.0.4103.61
LG V30+ // Android 8.0 // Google Chrome 81.0.4044.138

Pushed fix to master branch.

The "We can’t access your camera" pop-up is displayed
Checked on DEV V0.22.1-1
Reproduced on:
Desktop: Windows 10 // Google Chrome 83.0.4103.61
LG V30+ // Android 8.0 // Google Chrome 81.0.4044.138


Fix test on symfony-44 test on merge of symfony 4.3 + symfony 4.4 ruleWithConfiguration()

@TomasVotruba @lucasmirloup this is based on fix of PR:

https://github.com/rectorphp/rector-src/pull/4831

temporary use rector-src:dev-failing-fixture for it.

Oh, it's not, symfony-44 is not merge of symfony-43 and symfony-44 so when enter new test class, it should reset the Option::RECTOR_CONFIGURATION values. Closing for now.


Was there a reason why you didn't inherit nn.Module in ImagePyramid class?

I found out that you've not inherit nn.Module in ImagePyramid.
was there a specific reason for this?
thanks!

We did not have any trainable parameter in ImagePyramid, so we did not inherit nn.Module.
Thanks


ci(workflow): ignore tags & dependabot push

:tada: This PR is included in version 4.0.8 :tada:
The release is available on:

v4.0.8
GitHub release

Your semantic-release bot :package::rocket:


make prometheus client version same with tidb

change github.com/prometheus/client_golang to v0.9.0 and go mod tidy
PTAL @coocood

LGTM


EXTREME TPS LAG

I've been going around trying to get to the bottom of the TPS lag that is occuring on a server I am running. This is the third and final place that I've come to and considering I can't even find a discord for the forge port makes all this so much harder as the origins fabric discord doesn't provide support for forge. TPS spikes hard as more and more people join and it doesn't recover unless people leave. For some reason origins is executing a massive amount of recipe/inventory related commands and I have 0 idea as to why. It is currently taking up 50% of the entire server thread by itself and its ridiculous. I have a few screenshots of what is happening, do you have any idea what this might be?
Server Thread pic 1
Server Thread pic 2
Additional Entity Attributes (origins forge only view) image 1
Additional Entity Attributes (origins forge only view) image 2

There is no discord for forge as community management is absolutely not one of the skills I have.
Origins doesn't run the clear command, at least not with default origins, that's a datapack oversight.
The way recipe lookup is implemented is bad, as in doubles recipe lookup time bad. I'll try to not lookup recipes if the player doesn't have one of these powers, but that has a cost too.

I have no origins datapacks on the server i am running and additionalentityattributes is also using a large portion of my server

That's not aea, if you look at the namespace it's apoli.

If you can, I'd be interested in the stacktrace for IPowerContainer.hasPower. There might be some things that can be done there also.

@EdwinMindcraft how can I go about getting that for you?

Same thing you did for your first picture (Server Thread pic 1), just expand the tree as much as possible.

I can't find it in all view

Right now TPS is fine so I cant run a profile

Don't worry, what you've given me is basically what I wanted. I think I have an idea to make it run faster, at the cost of a bit of memory.

Alright, thank you!

Has this been worked on?
I am experiencing TPS lag, and by using Spark I found out it was mostly Apoli with MineColonies' citizens.
It was checking all the items in those citizens, making copies of each item along side with their capabilities, which apparently uses a lot of tick time. (EntityLinkedItemStack)
Beside checking items in living entities, it is also using a notable amount of tick time checking PhasingPower and ClimbingPower for living entities. (GlobalPower)
Is it possible to add a switch to disable all those parts if no registered power uses those (EntityLinkedItemStack and GlobalPower)?

Has this been worked on?
I am experiencing TPS lag, and by using Spark I found out it was mostly Apoli with MineColonies' citizens.
It was checking all the items in those citizens, making copies of each item along side with their capabilities, which apparently uses a lot of tick time. (EntityLinkedItemStack)
Beside checking items in living entities, it is also using a notable amount of tick time checking PhasingPower and ClimbingPower for living entities. (GlobalPower)
Is it possible to add a switch to disable all those parts if no registered power uses those (EntityLinkedItemStack and GlobalPower)?

Your issue is different to what's being caused by this issue.
And I do have plans to optimise EntityLinkedItemStack next update.


Explore whether adding a QR code scanning option in the Wi-Fi setup screen is possible

In the Wi-Fi setup screen, there could a QR scanning button to be able to add the Wi-Fi network in that way, simplifying setup. This is likely only desirable if it's simple to implement and maintain.

the feature exists already, but in a different way.
QR scanner button is available in the "Add network" activity (the activity that comes up after clicking on "+  Add network" button).


Include daily corrections in case data

The corrections of the case data from rki of the current data are exculded by setting the entries of AnzahlFall, AnzahlGenesen or AnzahlTodesfall zero where NeuerFall, NeuGenesen, respectively NeuerTodesfall is negative. It should be tested whether those corrections are not included already into the data from rki. If not, those corrections should be included directly into the case data.

Those corrections are already included from rki into the data on the date where the negative value appears. So if they would not be set to zero, they would be subtracted twice.


Add/create icon for pagination component

We need a fancy new icon for pagination on the components page.

We may have a challenge differentiating this new icon from the existing "Direction Links" icon