Add support for signing in using multiple accounts

Add support for signing in using multiple accounts. MSAL supports this capability natively so we can build on top of it.

[x] research if switching accounts is possible between app-only and delegated or only delegated
[ ] change the login command so that it doesn't log out previously signed in user. Also, after signing in, it adds the identity to the list of available identities (if it wasn't already present in the list) and sets this identity as active
[ ] extend the logout command with the ability to specify account that should be signed out. If not specified, log out from all accounts. If you logout from a specific account, leave the CLI in identity-less state. CLI will prompt you to login or select an identity (if available)
[ ] add a new identity list command to show a list of all signed in users (m365 status will keep returning the currently active identity)
[ ] add a new identity set command to select which identity to use: m365 identity set --identity <identity>, where identity is a human-readable identifier of the signed in identities to choose from. The selected identity get set as active and will be used by CLI when running commands. If user selects an invalid identity, we keep the previously selected identity as active.
[ ] extend the auth logic to use the identity that's currently set as active
[ ] extend the debug output to log the human-readable identifier of the identity used to execute the command

m365 identity set specs
When signed into multiple accounts, switch to another account
Usage
m365 identity set [options]

Options

Option
Description

-i, --id [id]
The Id (GUID) of the identity to switch to. Can be found by running m365 status. Specify either id or name but not both.

-n, --name [name]
The name of the identity to switch to. Can be found by running m365 status. Specify either id or name but not both.

Remarks

The Id used here is the localAccountId as MSAL returns it. That way we can find the account to logout from. My proposition is to add that localAccountId as an extra property identityId on the m365 status output. This id should also be saved in our cache, so we we know what MSAL account to switch to and logout from.

The name property is the value as visible in the connectedAs property when running m365 status. It can be a UPN or the application name.

Prompts: When not specifying an option, the user will be prompted to select an identity using our inquirer prompts. For this to happen, the prompt config key will need to be enabled, otherwise a validation error will be thrown. This concerns a new prompting feature and is described in https://github.com/pnp/cli-microsoft365/issues/5560.

Failures: When the command fails, the user should be put into an identity-less state, otherwise side effects might occur in scripts where people expected an identity to be selected, while in fact the previously selected identity is used.

Note: We currently have two caches: 1) The MSAL token cache which is saved to a file .cli-m365-msal.json and b) Our own cache which saves the auth.service object and is saved to .cli-m365-tokens.json. Because we are now able to log into multiple accounts, we should save the localAccountId, as a link between both caches. If we do it like this, the caching can just remain the same.

Examples
Switch to a user account by name:
m365 identity set --name<EMAIL_ADDRESS>
Switch to an application account by name:
m365 identity set --name 'My application'

Switch to an account by id:
m365 identity set --id '0bb7cb89-7fae-4775-a01a-c372cc167371'

m365 logout specs
Usage
m365 logout [options]

Options

Option
Description

-i, --identityId [identityId]
The optional Id (GUID) of the identity to logout from. Can be found by running m365 status. Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from.

-n, --identityName [identityName]
The optional name of the identity to switch to. Can be found by running m365 status. Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from

Examples
Log out from all accounts:
m365 logout

Log out from a single (user) account by name:
m365 logout --identityName<EMAIL_ADDRESS>
Log out from a single application account by name:
m365 logout --identityName 'My application'

Log out from a single account by id:
m365 logout --identityId '0bb7cb89-7fae-4775-a01a-c372cc167371'

m365 identity list specs
Show the list of currently signed in identities
Usage
m365 identity list [options]

Options
No options
Examples
Returns a list of currently signed in identities:
m365 identity list

Discussed in https://github.com/pnp/cli-microsoft365/discussions/3453

Originally posted by oweiler June 29, 2022
In my quest to get rid of all basic auth usage inside my Microsoft 365 tenant, I happened upon this project as a potential solution.  Most of my basic auth usage revolves around sending and receiving emails in exchange which this project provides.  I'm now looking to extend this functionality to multiple Azure AD identities/exchange mailboxes but use them all within a single linux login.  Using basic auth, this was relatively simple: keep protected files with usernames/passwords as resources to curl to get and send mail from/to that mailbox.
With cli-microsoft365, reading about persistent connections, I'm not sure it's that simple?  One way I could see doing what I need is to keep a version of .cli-m365-msal.json and .cli-m365-tokens.json for each mailbox/M365 identity I want to manipulate but this has the obvious drawback that I can only perform operations on a single identity at a time.
I looked into the options provided by the m365 login command but they all seem to end up in a place where the .cli-m365-msal.json and .cli-m365-tokens.json point to a single Azure AD identity.
What's the right way to handle multiple Azure AD identities from a single login/user on a linux system?

Nice suggestion @waldekmastykarz some thoughts from me.

m365 account set --identity <identity>
Did you mean m365 identity set?

Where identity is a human-readable identifier of the signed in identities to choose from

Could we extend the login command with an optional --identityName property, so the user can define a name that is relevant to them?

change the login command so that it doesn't log out previously signed in user. Also, after signing in, it sets this identity as active.
This might be assumed but the login command should add the identity to a pool of signed in users

Did you mean m365 identity set?

Yes! Updated

Could we extend the login command with an optional --identityName property, so the user can define a name that is relevant to them?

Possibly. I'd suggest that we first look at what's available to us from MSAL and if that's sufficient or if we need something on top. I wonder if something like user name or app name would be sufficient, or maybe even we could make the identity set command interactive so that you can select the identity from the list and don't need to memorize anything.

This might be assumed but the login command should also add the identity to a pool of signed in identities.

Yes

I like it @waldekmastykarz.
So when you have logged in to four accounts and you log out of the active identity, what will m365 status return?

Good question @martinlingstuyl! We can either pick one from the list (last or first for example) or introduce another status and prompt user to select an identity. Which one do you think would be more intuitive?

I think it would be best to not auto-select another identity. This could be rather dangerous 😀

Good point. I'll update the spec.

Also: how about interactively selecting an identity from a list using the arrow and enter keys?

Also: how about interactively selecting an identity from a list using the arrow and enter keys?

That's exactly what I meant. Let's see if we can do this using inquirer which we already use for prompts.

Sorry for the late response.
I like the idea a lot 👍. Seems like it would give a lot of flexibility.
I wonder if we could introduce a new option which would apply to all commands (like --debug) which would allow to change the identity along the way executing the command, like --identity. I wonder if this would not give even more flexibility if user could just (optionally of course) specify some identity in one command and a different one in a second command, and CLI would do all the switching and executing the command to the proper place 🤔.
... But ok, this I would leave for the future 😉.

Interesting idea @Adam-it. What would be the use case for it?

I like your idea @Adam-it, if we were to take a use case of sending an email using different identities, you could do this with three commands.
m365 outlook mail send ... --identity <identity>
m365 outlook mail send ... --identity <identity>
m365 outlook mail send ... --identity <identity>

As apposed to using the proposed identity set command.
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>

I think there is room for both options here.

@waldekmastykarz sorry for the not sufficient response. I am currently on holiday mainly over a phone 😋. I will try to catch up over the weekend when I will be two days out of the woods and next to some laptop 😅.
.... anyway. What @garrytrinder described is exactly what I had in mind. Good work @garrytrinder reading my thoughts ...magic powers I guess 🧙♂😉.
@waldekmastykarz there is no really good use case except that it may be more convenient for the user to do all in single line instead of two lines 😅. So it is really something nice to have we could introduce later on 👍.
I had something in mind like we have a lot of commands we do in context of identity A but in the middle of this all we want to execute a command with identity B 😉.
Like :
m365 spo file add ....
m365 spo folder rename...
m365 spo listitem remove....

// TODO: send mail using different identity. and then move on with other stuff. I would need to do 3 lines identity set -> send mail -> identity set back to previous one. Alternatively I may pass identity  as option to any command which will run this one command only with different identity (the passed one). In background CLI actually would use the identity set command to manage this but the user may just use one liner 😉
m365 outlook mail send ... --identity <identity>

m365 spo list list....
etc...

I hope it's more understandable now.. I did my very best 🤩.... better I may do on weekend 😋

m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>

...might not even work reliably, because it sets identity system-wide, so if you were to run something else in parallel, you might get mixed identities.
I'd say that this is a separate use case that we investigate once we have the basics implemented.
Also, when we think about introducing a global option, we need to consider it being available on all commands, like login, status, spfx *, cli completion * etc. so we need to give it a proper thought if that's something that we really want.

I'd say that this is a separate use case that we investigate once we have the basics implemented.

I'm happy with this.

Love the idea about multiple identities ❤. Super handy for the consultants who handles gazillion of accounts and does IT Pro operations for them.
As an extension to the command, I was thinking whether we can  show which identity was used for executing command.
As a user who manages multiple tenants, and when I execute, I would love to see and confirm whether the command is executed against the right tenant. Even now the same issue is there, but when we give the support of concurrent account sign-in, probability of execution against wrong tenant would be on a higher end.
My idea would be something like this

m365 teams team add --name "Architecture" --description "Architecture Discussion"
Executing using the identity<EMAIL_ADDRESS>@odata.context        : https://graph.microsoft.com/v1.0/$metadata#teams('7f0d5bfc-fae5-43c0-8bdf-030916daab70')/operations/$entity
Value                 : {"apps":[],"channels":[],"WorkflowId":"northcentralus.ff8de907-cd7e-4505-a165-b38ad8373d18"}
attemptsCount         : 1
createdDateTime       : 2022-08-19T11:22:38.166614Z
error                 : null
id                    : f9374ac4-3c1a-427e-85bf-3e4c56a3423d
lastActionDateTime    : 2022-08-19T11:22:38.166614Z
operationType         : createTeam
status                : notStarted
targetResourceId      : 7f0d5bfc-fae5-43c0-8bdf-030916daab70
targetResourceLocation: /teams('7f0d5bfc-fae5-43c0-8bdf-030916daab70')

Used Identity Prompt could be made as a configuration so that it could be disabled via settings. This may not be relevant for the scheduled script usages, but would be handy for IT pros who does execution against multiple tenants.

As an extension to the command, I was thinking whether we can show which identity was used for executing command.

Great idea! We should include it in the debug output. I'll add it to the spec.

As a user who manages multiple tenants, and when I execute, I would love to see and confirm whether the command is executed against the right tenant.

How would that work in a script, where each command would prompt you to confirm the identity that you want to use if there are multiple signed in?

Great idea! We should include it in the debug output. I'll add it to the spec.

That will be wonderful ❤

How would that work in a script, where each command would prompt you to confirm the identity that you want to use if there are multiple signed in?

Here my thought process is show this output always when the output type is text.
So my understanding in the whole process is that, when you execute m365 identity set --identity <identity>, all the commands which follows after that will use that identity as default.
So that being the case, my thought process was to log the message Executing as <identity> to the console and then show the output of the command. This would be the case only when the output is text. For JSON, its flow will remain as usual.
 Message Executing as <identity> will be shown on the console only when the flag is enabled.
Am I making sense?
If I have understood the process wrong and If the process is that - Each execution will prompt for identity, then above scenarios will not stand and does not make sense.

So that being the case, my thought process was to log the message Executing as  to the console and then show the output of the command. This would be the case only when the output is text. For JSON, its flow will remain as usual.

Right, but that's just a log message, not a prompt, right? We could print it even in JSON output. As long as we send it to stderr it won't be mixed up with the command's output. We'd need to check though how it would work in PowerShell that has its own notion of output streams.
Would we show this message always, or only if there are multiple identities signed in? Also, would we show it always or only in verbose and debug output?

Right, but that's just a log message, not a prompt, right?
Correct. It is just a message which shows before the execution of the command and gets console log.

Would we show this message always, or only if there are multiple identities signed in?
My vote would be always irrespective of multiple identities to make it consistent irrespective of how many identities are available.

Also, would we show it always or only in verbose and debug output?

My vote would be to show it always in the verbose and debug. And in normal mode, it will show only if the config setting alwaysShowExecutingIdentity is true. Otherwise it will not be shown

Right, but that's just a log message, not a prompt, right?

Correct. It is just a message which shows before the execution of the command and gets console log.

Would we show this message always, or only if there are multiple identities signed in?

My vote would be always irrespective of multiple identities to make it consistent irrespective of how many identities are available.

Also, would we show it always or only in verbose and debug output?

My vote would be to show it always in the verbose and debug. And in normal mode, it will show only if the config setting alwaysShowExecutingIdentity is true. Otherwise it will not be shown

Hey @pnp/cli-for-microsoft-365-maintainers, what do you think about Arjun's proposal to always include identity information?

I think it might give a lot of clutter in the logstream, on the other hand I agree that this can be life-saving information.
Two thoughts:

@arjunumenon 's proposal: if the information is only available on verbose and debug, we'd still risk missing it. I'm usually only setting verbose or debug in specific scenarios where I want to see what happens. Might it be better to always show the warning line in the logstream, unless you've set a hide... config key?

another proposal would be to show a warning line when executing the first m355 command in a given shell session, or the first command after using identity/account/login commands. This would reduce the clutter but still alert the user.  I'm not sure if it's possible though. @waldekmastykarz, do you think that can be done?

+1 to always add in verbose and debug and to normal output when some config setting set to true which by default is false.
@martinlingstuyl very interesting idea with showing the warning message when first command executed 🤔. Should it only be shown when we have more than one identity logged in?. What if along the way we add or logout from some accounts. Should this warning be shown every time something changes?. TBH I still believe we may have scenarios that the user may not see this warning as it got lost along the way of work 🤔

another proposal would be to show a warning line when executing the first m365 command in a given shell session, or the first command after using identity/account/login commands. This would reduce the clutter but still alert the user. I'm not sure if it's possible though. @waldekmastykarz, do you think that can be done?

Unfortunately, each CLI command is run standalone, and shells have no notion of a session, so I don't think we can do this.

which would allow to change the identity along the way executing the command, like --identity. I wonder if this would not give even more flexibility

Yesterday I ran into the need for this: I'm used to running a single script in multiple posh windows to update a lot of sites as fast as possible. This is a possibility with PnP PowerShell because the sign in session is restricted to the PowerShell terminal session. I can sign into multiple posh terminals using multiple identities and update the sites, not having to worry about throttling.
For the CLI this is currently not possible, as we don't support multiple identities and as the sign in session is system wide.
I'd like to be able though, to sign into multiple identities and being able to use an --identity flag on every command so I can use the CLI for this as well as PnP Powershell.

I'd like to be able though, to sign into multiple identities and being able to use an --identity flag on every command so I can use the CLI for this as well as PnP Powershell.

I still have my doubts about this approach. I think it would be preferable to set the identity on a session and we should look for ways to do that.

I think it would be preferable to set the identity on a session and we should look for ways to do that.

One way I just realized that would be possible is to assign the identity to the PID of the current shell process which we can get from process.ppid. That way we could imitate a "session" and store all kind of information applicable only to the current terminal process.

One way I just realized that would be possible is to assign the identity to the PID of the current shell process which we can get from process.ppid

Freakin' awesome 😁 (And persisted in the GitHub issue/process)

Thinking some more about the logout command, I wonder if it wouldn't be more in line if:

when you type logout without options, CLI logs out the currently active identity and sets the CLI in identity-less mode
when you type logout with a specific identity, CLI logs out that identity. If it happens to be the currently active identity, see above
when you type logout --all CLI logs out all identities

Here's some recent findings:

research if switching accounts is possible between app-only and delegated or only delegated

That's not possible using MSAL. MSAL bases its multi-account support off of the client, and public and confidential clients are two different clients, so if we want to support not only multiple accounts but also multiple accounts, we'd need to handle it ourselves. With MSAL, we'd only have the ability to store multiple user accounts for the same client and the moment we use a different client (AAD app), we'd need to destroy the previously stored client and account information.
Since you can use any AAD app with the CLI, I suggest that, if we want to proceed with this functionality, we handle support for multiple accounts ourselves. We'd store all information about the different apps used with the login command, and then let users select which identity they want to use.
Thoughts @pnp/cli-for-microsoft-365-maintainers?

Ah, awesome that you dug into this.
So basically you're saying that if you want to use multiple apps to sign in, this would not be supported by the MSAL setup for multiple accounts?
Using multiple apps is a pretty often used scenario on my machine, where im signing into m365 using the default app as well as custom apps with certificates.
I'd like it to be as flexible as possible here. So creating our own setup would be the way forward as far as im concerned.
So, do we need to cache refresh tokens for this to work?

If possible, I'd go for full flexibility as well. Both delegated and app authentication together would be awesome.

Using multiple apps is a pretty often used scenario on my machine, where im signing into m365 using the default app as well as custom apps with certificates.
I'd like it to be as flexible as possible here. So creating our own setup would be the way forward as far as im concerned.
So, do we need to cache refresh tokens for this to work?

Thanks for bringing this up. This provides some extra important information for implementing this feature. We've been caching refresh tokens already, so this wouldn't be anything new for us. When implementing this feature, we'd build an extra management layer on top of MSAL, so that we can properly store multiple identities and after selecting one, pass it to MSAL for auth. For listing and selecting identities, we should then not only show the user name but also app- and tenant ID, auth type, and perhaps extend the login command to let users specify a user-friendly name to select the identity by.

Hi guys,
I've been experimenting with this feature as I for one could really use it. (And all people who work in similar IT Services)
I'm currently as far as being able to sign in with multiple accounts. 🚀

I'm a bit late to the party here but that SC looks awesome @martinlingstuyl! That's a feature I would love to see implemented. A question I've here. Why are we listing the other accounts in an array within the original object? Wouldn't it make more sense to return a list that lists all accounts and add a boolean property for which one is selected?

That would be one of the things we'd need to discuss indeed @Jwaegebaert.
For one, it would be a breaking change to give it back like that.
Also: in the code I'm trying to change as little as possible about the auth.service object, because that's used like everywhere :-) Working with an array would be annoying all over the place. We would need to change the code everywhere.
Also we're saving the service to a json file as a cache. That's an object as well. It would also be a breaking change if we switched to saving an array.
So I just chose the easy route for now:
I've added an otherAccounts property on the auth.service object to work with it.
I fear the refactor would be huge otherwise.
Of course the code structure would not need to define the response output 1 on 1, but then again: it's a breaking change there as well.

Would it make more sense if it read availableIdentities?

Extra argument for the response output here:
If you're passing the status into a variable, it's easier and more logical for the user to work with an object:
$status = m365 status | ConvertFrom-Json
write-host $status.connectedAs

m365 identity set specs
When signed into multiple accounts, switch to another account
Usage
m365 identity set [options]

Options

Option
Description

-i, --id [id]
The Id (GUID) of the identity to switch to. Can be found by running m365 status. Specify either id or name but not both.

-n, --name [name]
The name of the identity to switch to. Can be found by running m365 status. Specify either id or name but not both.

Remarks
The Id used here is the localAccountId as MSAL returns it. That way we can find the account to logout from. My proposition is to add that localAccountId as an extra property identityId on the m365 status output. This id should also be saved in our cache, so we we know what MSAL account to switch to and logout from.

Note: We currently have two caches: 1) The MSAL token cache which is saved to a file .cli-m365-msal.json and b) Our own cache which saves the auth.service object and is saved to .cli-m365-tokens.json. Because we are now able to log into multiple accounts, we should save the localAccountId, as a link between both caches. If we do it like this, the caching can just remain the same.

m365 logout specs
Usage
m365 logout [options]

Options

Option
Description

-i, --identityId [identityId]
The optional Id (GUID) of the identity to logout from. Can be found by running m365 status. Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from.

-n, --identityName [identityName]
The optional name of the identity to switch to. Can be found by running m365 status. Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from

Remarks on this @pnp/cli-for-microsoft-365-maintainers?

@martinlingstuyl why not in both places use the same option naming? I like identityId and identityName more then id and name. Seems more descriptive.
Also, why not name the command m365 identity switch? It won't be actually updating so set does not seem fit IMO 🤔.

As to the option names: that's because of our naming convention: not reusing the last noun in the option name.
As to switch: I just copied over identity set from the initial specs. I agree with you that switch sounds better, and it should be possible in such a case as this. What do you think @waldekmastykarz? (As you created the initial specs)

I'd avoid introducing additional verbs. set is how we're defining updating/activating throughout the CLI and I think it's still a good fit for this purpose.
@martinlingstuyl MSAL has native support for multiple accounts. Are you using it or have you chosen a different path? I wonder how much can we use off of it, rather than building it ourselves?

Hi @waldekmastykarz, yes I'm absolutely using MSAL native multi-account support here.
It's basically the MSAL Token Store that you can access through functions like getAllAccounts().
As I've written somewhere in my comment: we store the MSAL token store in a json file (.cli-m365-msal.json), and we also have our own extra json file (.cli-m365-tokens.json) with account-information relating to the CLI, for example the login method.
In my work with this, I've kept everything largely the same. The only thing I had to change was adding an extra ID (localAccountId) to our own json file store, to make sure we know how to get the correct account from MSAL to work with it.
And of course, i've updated our own .cli-m365-tokens.json json token store with an array of the additional identities.

For one, it would be a breaking change to give it back like that.

That is a fair point, it's also quite a common command that is used in scripts. So on the breaking change chain, it would be enormous.

Would it make more sense if it read availableIdentities?

That's a good suggestion. I'm more keen on this name.
Regarding the m365 identity set specs, I've got nothing to add to them. They look good to go. Also more of a fan of using set

Any other feedback @pnp/cli-for-microsoft-365-maintainers?

For the status command, would it make sense to keep its output as-is and then introduce another identity list command that lists available signed in identities so that the whole feature is implemented in a set of commands that are grouped together by name?

That's perfectly fine as well 👍 would be similar to how az works

Ok, I've updated the issue specs. Time for a last review @pnp/cli-for-microsoft-365-maintainers

Just to check: does MSAL also supports multiple apps signed on simultaneously using app-only auth?
One more thing that popped to my mind. Consider running a script that you want to ensure is running under a specific identity. Right now, you'd have to run the status command and compare the name of the currently active identity with the intended one, which adds complexity. Alternatively, you'd call identity set specifying your identity and the name/id you specified is not available in MSAL cache, I suppose the command and hence the script would fail acting as a failsafe. I think this is something that we should document somewhere to help people build robust scripts and avoid side-effects.

I'd need to check that: haven't tried that yet!
Plus a good point on documenting!

Alternatively, you'd call identity set specifying your identity and the name/id you specified is not available in MSAL cache, I suppose the command and hence the script would fail acting as a failsafe.

Good point on this. In scripts, the script could continue executing on failure of identity set. Meaning it could execute actions using the previously selected identity.
If identity set fails, we should therefore probably set the cli to an identityless state, to avoid this side effect.

I've created another issue to describe enhanced prompting functionality that we'd need on this issue as well:
https://github.com/pnp/cli-microsoft365/issues/5560

@martinlingstuyl since you are already working on this one I will align the tags if that's fine with you 👍

I certainly am! Ill probably create a PR next week...

Ok people, @waldekmastykarz and I have discussed multi-accounts in some more detail. And we'd like to propose a change to the specs:
The idea is that we don't implement identity set and identity list as new commands, but include the functionality in existing commands.
Also we'd want to propose renaming m365 login and m365 logout to m365 connect and m365 disconnect. (keeping login and logout as aliasses)
Central in our discussion was the idea of having connections with m365 instead of identities or accounts.. A connection encompasses more than an identity, as it can have certain properties, like the manner of authenticating. (certificate, secret, etc) and it's identity information, like Id and name. Which we already employ. connect and disconnect seem to fit better with the thing we're trying to accomplish.
In this new setup, you can use m365 connect for the initial connection, but also to switch to another connection:
# Connect to m365 by signing in using the default deviceCode flow

m365 connect

# Switch to the connection that's signed in with<EMAIL_ADDRESS>
m365 connect --identityName<EMAIL_ADDRESS>
# Disconnect from m365

m365 disconnect

# Disconnect a specific connection

m365 disconnect --identityName<EMAIL_ADDRESS>
Listing connected identities
To get at the identityId and identityName you can run m365 status
It will include an array of all identities. But just the Id and name properties.
prompting in interactive mode
An optional extra would be that running m365 connect with no options, may show a prompt where you can choose another connection or the intent to connect using another identity.
m365 connect
You have multiple active connections to Microsoft 365.  Do you want to switch to one of these identities or connect using a new identity? Select one of the options.
><EMAIL_ADDRESS>><EMAIL_ADDRESS>> My Custom Application
> Connect using a new identity

What would you say of such a change? And @waldekmastykarz, reading it written out like this, do you still agree?

Great summary and still fully on board @martinlingstuyl 💪

hmmm not sure 🤔.
I understand the concept and I like the idea and the simplicity and also the fact we would not have the additional identity commands.
My only concern is that the naming is very similar to the PnP PowerShell Connect command that allows the user to connect to a specific SP Site.
For us is plain simple and makes sens. For someone new that is used to PnP PS and now would be using CLI for M365 for the first time it will be very misleading I guess. With the login and logout we didn't have this problem as the naming was totally different.

My only concern is that the naming is very similar to the PnP PowerShell Connect command that allows the user to connect to a specific SP Site

I understand what you mean. However, the likeness is quickly gone once you realize there is no SharePoint site URL option.
It then becomes more like 'm365 connect': I'm connecting to m365!

Other input @pnp/cli-for-microsoft-365-maintainers?

Nice work with the research on this topic. I'm also not really keen on changing m365 login to m365 connect. This is one of the most used commands and changing the name of this would be a huge breaking change, although you're keeping it as an alias. I understand that we can do changes over time to command names but this feels like we're changing for the sake of introducing a new breaking change. I'm not quite sure why we should rename this.
About the idea of introducing a new option to --identityName is a solid plan. this would make the commands login, logout, and status a complete package to quickly manage and change your identities. All I can say is, ship it 😄

I'm not quite sure why we should rename this.

So you're saying you don't yet see the logic, or the necessity for this?
You could say that If we add identityId and identityName to the login command, it becomes less logical to call the command login, as we've already logged in. We're not signing in again. It's slightly more logical to say you are "connecting to microsoft 365 using an identity that we already know". m365 connect is slightly more generic and does not express the intent to go throught the login flow perse, like m365 login does.
But maybe it's a bit 'poteto potaato'. @milanholemans, @appieschot, what are your views on this?
(In v8 we would stop supporting login of course. But to ease the process of transitioning, it's not a bad idea if we show a warning message when people do use m365 login.)

I'm not really convinced to rename these two commands, to be honest. Maybe it makes more sense to rename it to connect/disconnect (not sure about that yet), but in my opinion login and login are just as clear. And for new people much easier to understand. To me, it looks like a quite drastic breaking change.
Instead of m365 connect --identityName abc, I would prefer m365 login --connectionName abc or something like that.
It would be a great enhancement if you could pass a name (not sure if this should be optional or required) for a connection. When you have different customers, giving your connection a unique name is much easier. Instead of connecting to m365 connect --identityName<EMAIL_ADDRESS>I rather run m365 connect --identityName contoso to switch identity.

Not to mention the number of sample scripts we would need to update when v8 is out... like all of them 😅, at least in our repo where we try to keep up to date with scripts with the latest major version.
What if we keep both? login and connect? That's probably the worst option right 😅 as it does not solve the problem of naming and creates more confusion 😉

I'm also thinking of all the sample scripts. We always have to keep them up to date with major versions but changing the login command would impact them all. From a naming perspective, it makes sense but then the approach of @milanholemans could also be a route to take. Changing the option name to connectionName rather than identityName.
+1 on the suggestion to allow passing a connection name. That would make it even easier to switch.

To clarify the rationale behind the proposed rename. It's anything but renaming for the sake of rename. The last thing we want to do is to break our users and we've got some good first hand experience of what impact a breaking change in a dependency has, especially at scale.
We want to change so that the action sound logical when you read it. m365 login --connectionName .... Are we saying that you're logging in with a connection? That doesn't make any sense, doesn't it? Also, we're not naming connections. We have identities which have an ID and a name (upn or app name). Connection is a combination of identity and a way to authenticate (cert, device code, secret, MSI, etc), along with some state information such as refresh- and access tokens.
m365 login --identityName xyz - but you've already logged in with that identity and are merely activating it, confusing, no?
We can always go back to m365 identity set --name xyz, but isn't that complicating it, comparing to m365 connect --identityName xyz?
Yes, renaming login to connect is a breaking change that would affect everyone. But I think the main question is will it make life easier going forward?
I appreciate your perspective and let's keep 'em coming. This would be a serious change so let's weigh all the options before we commit so that we're sure we're on the right path.

We want to change so that the action sound logical when you read it. m365 login --connectionName .... Are we saying that you're logging in with a connection? That doesn't make any sense, doesn't it?

I agree that when you read it, connect would make more sense yes. But I think something like m365 login --connection abc or something like that is just as clear. You interpret it like "I'm logging into CLI with an existing connection".
Anyhow, I can live with it when we rename the command, but in my opinion, for new users login is more clear than connect. On the other side you have quite some PS modules that use connect to login, e.g. Connect-PnPOnline, Connect-SPOService, Connect-ExchangeOnline, Connect-AzureAD, ... So, in that case, I can accept the rename if we decide to do this.

Also, we're not naming connections. We have identities which have an ID and a name (upn or app name). Connection is a combination of identity and a way to authenticate (cert, device code, secret, MSI, etc), along with some state information such as refresh- and access tokens.

Yes, but in my opinion, it would be cool if we could give every connection a custom name to easily switch. As mentioned before, it's like giving a name to your credentials in Windows credential manager. That makes it easy to target a credential rather than typing the entire email.
Instead of:
m365 connect --identity<EMAIL_ADDRESS>m365 connect --identity<EMAIL_ADDRESS>
The following will be more user-friendly:
m365 connect --identity contoso
m365 connect --identity "contoso admin"

Besides that, if the identity is only a UPN or app name, we might run into trouble. For multi-tenant Azure AD apps using application permissions, it's possible that 2 or more connections would have the same app name.

...for new users login is more clear than connect...

Unfortunately, I kinda have the same feeling 🤔.
Basically, I am also fine with renaming even if this means a lot (.. ok some) work with refactoring the scripts. My comment was only to point out that we should think what is best for the ned user (this is not always the most logical option as it may turn out 😅).
Also you may 'connect' to many CLIs but only 'login' to the best one 😉

Besides that, if the identity is only a UPN or app name, we might run into trouble. For multi-tenant Azure AD apps using application permissions, it's possible that 2 or more connections would have the same app name.

In that case it will show our disambiguation prompt.

In that case it will show our disambiguation prompt.

How will you differentiate? By showing the tenant ID? That doesn't say a lot according to me.

That's same challenge for all disambiguating prompts.
It is possible to change that I guess.

To answer some of the points that are made:

We could also leave m365 login as a permanent alias, (beyond v8) if you're worried about new users. For this command I'd not mind if we do. We may of course point to m365 connect as the primary route.

In case the user needs to disambiguate the required identity from a list of identities with the same name, currently we write ID's to the screen (as we do for all disambiguation prompts). It's true that does not always help a user very much. For the identity disambiguation prompt this would be the same. Maybe we can discuss separately how we can show a more useful selection.

how do you guys feel about this, after having thought about it a bit. Do I have your blessing in implementing it this way?

Personally, still not keen on renaming it to connect. If we change it, I suggest starting it off as an alias and awaiting some feedback from the community. Just so we can gain some other perspective from folks who are using it intensely.
Giving the identities an alias would be a nice enhancement but like you're suggestion @martinlingstuyl. That shouldn't be implemented straight from the bat but would be a nice addition for a separate issue.

If we change it, I suggest starting it off as an alias and awaiting some feedback from the community

I don't think that's really an option. m365 login --identityId "<guid>" is just quite unclear. We'd best think about the most logical direction forward that fits all the purposes we see the CLI used for. I believe we should either move to the connect-concept or build it as m365 identity set. (A shorter alternative could also be m365 switch. )
As said before: for people coming from PowerShell commandlets, (which is most of our users), m365 connect will definitely make sense as well as m365 login does. I know some other CLI's use login, like az login, but then in their implementation of multi-account they have exactly the command sprawl that we're trying to avoid: az account show / az account list / az account set etc etc
I believe using m365 connect / m365 disconnect / m365 status we keep a clear single set of commands for working with auth.

We could also leave m365 login as a permanent alias, (beyond v8) if you're worried about new users. For this command I'd not mind if we do. We may of course point to m365 connect as the primary route.

TBH this is quite unclear but works for me 😅. at least for now. That way we may add the connect and root users to this path but still keep the old login as safe option

In case the user needs to disambiguate the required identity from a list of identities with the same name, currently we write ID's to the screen (as we do for all disambiguation prompts). It's true that does not always help a user very much. For the identity disambiguation prompt this would be the same. Maybe we can discuss separately how we can show a more useful selection.

yes let's create a separate issue for it. I would love to take it up 👍 and improve this 👍

I've updated the specs @pnp/cli-for-microsoft-365-maintainers


Add peer option to force RPKI valids

A dicussion in networking-ix brought up the fact this feature should probably exist to allow for better filtering of downstreams. I hope I didn't forget anything required

Codecov Report

Merging #66 (21cea16) into main (63ac3eb) will decrease coverage by 0.21%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
- Coverage   40.74%   40.52%   -0.22%
==========================================
  Files          17       17
  Lines         945      945
==========================================
- Hits          385      383       -2
- Misses        471      473       +2
  Partials       89       89

Impacted Files
Coverage Δ

internal/config/config.go
58.10% <ø> (ø)

internal/bird/bird.go
21.87% <0.00%> (-3.13%)
:arrow_down:

Continue to review full report at Codecov.

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

Looks great overall. This is definitely a neat feature for certain use cases. The one open discussion item is the correct name for this kind of "strict" validation. I'm not sure the right answer to this so open to ideas.

I checked with a few others, and there doesn't seem to be a universal term for filtering unknowns. Strict RPKI makes sense to me, or even something as plain as rpki-reject-notfound


\complement doesn't parse correctly

The parser doesn't seem to handle Set complement correctly:

This appears to be A^\complement, which the latex parser interprets as:
[ 'Power', 'A', '\complement']
The other approach we considered was simply A\complement -- but the parser interprets that as:
['Complement', 'A', 'Missing']

The parser now handles \complement as an exponent.


Avoid re-downloading the model files

The llama2 model is very large. When I loaded it using transformers on my local computer, I set the cache_dir, so I have the complete model cache. Could you please guide me on how to configure the parameters when using SFTTrainer to make use of this cached model and avoid re-downloading the model files from Hugging Face?

We are using the same transformers logic to download/cache the model so I believe if you cached it once it should work unless you changed the cache location? Can you confirm that this is not the case?

My apologies, I misunderstood earlier. I thought the 'cache_dir' had to be set in SFTTrainer, but it turns out model instantiation is still done through AutoModelForCausalLM, just like before.

So it is working? In that case can we close the issue?

thank you very much


Distribute a pre-populated 'Favourites' m3u playlist

To showcase musicbox-webclient's 'favourites' feature, as discussed in https://github.com/pimusicbox/mopidy-musicbox-webclient/issues/167
Need to do our best to make sure all the streams included in the playlist work for all users (i.e. not geolocked and provided by a reliable station/service).

Closing in favour of the more descriptive issue at https://github.com/pimusicbox/pimusicbox/issues/377


CLI args to run cypress locally in run mode does not work from TestAutomation.md

Is there an existing issue for this?

[X] I have searched the existing issues

Description
The command written in TestAutomation.md doc seems to be deprecated as in latest version of cypress there is no Integration folder.
Steps To Reproduce
Goto https://github.com/appsmithorg/appsmith/blob/master/contributions/docs/TestAutomation.md
Check this

For run mode, you can use CLI args. Example: To run the entire Regression_TestSuite, use $(npm bin)/cypress run --headless --browser chrome --spec "cypress/integration/Regression_TestSuite//"

Public Sample App
No response
Environment
Release
Issue video log
No response
Version
Cloud

I think the command should be updated to

$(npm bin)/cypress run --headless --browser chrome --spec "cypress/e2e/Regression//"
Let me know if this make sense, I will update the doc.

Hi @somangshu, does this change make sense to you?

Tagging relevant folks from the internal team.
@Aishwarya-U-R @Parthvi12 would you be able to help here?
cc @rajatagrawal

@harshitpandey0426 Thanks for contributing, please go ahead with your change.

@harshitpandey0426  We have also moved away from app/client/cypress.json to app/client/cypress.config.ts. Will you be able to update that as well? Thanks!


Proposal: Allow configuration of the docker daemon to disallow unqualified repositories

This is the first half of my original Proposal at https://github.com/docker/docker/issues/1988#issuecomment-57261658
Proposal
Allow configuration of the docker daemon to disallow unqualified repositories.  This could be configured on a per command basis, such as:
--force-fully-qualified=push,login
--force-fully-qualified=pull,search
--force-fully-qualified=all

The additional overhead of longer image names could be mitigated by #8329, possibly in combination with #6805
Expected behavior:
Pull behavior
docker daemon not running with --force-fully-qualified=pull
$ docker pull debian
Pulling repository docker.io/debian
... [pull] ...

$ docker pull docker.io/debian
Pulling repository docker.io/debian
... [pull] ...

$ docker pull dkjer/publicapp
Pulling repository docker.io/dkjer/publicapp
... [pull] ...

docker daemon running with --force-fully-qualified=pull
$ docker pull debian
2014/10/01 02:35:45 Error: Missing registry name, try "docker.io/debian" instead

$ docker pull docker.io/debian
Pulling repository docker.io/debian
... [pull] ...

$ docker pull dkjer/publicapp
2014/10/01 02:37:52 Error: Missing registry name, try "docker.io/dkjer/publicapp" instead

$ docker pull docker.io/dkjer/publicapp
Pulling repository docker.io/dkjer/publicapp
... [pull] ...

$ docker pull <IP_ADDRESS>:5000/don/privateapp
Pulling repository <IP_ADDRESS>:5000/don/privateapp
... [pull] ...

Push behavior
docker daemon not running with --force-fully-qualified=push
$ docker push debian
2014/10/01 03:46:16 You cannot push a "root" repository. Please rename your repository to <index>/<user>/<repo> (ex: docker.io/dkjer/debian)

$ docker push dkjer/publicapp
The push refers to a repository [docker.io/dkjer/publicapp] (len: 1)
Sending image list
Pushing repository docker.io/dkjer/publicapp (1 tags)
... [push] ...

docker daemon running with --force-fully-qualified=push
$ docker push debian
2014/09/30 02:04:00 You cannot push a "root" repository. Please rename your repository to <index>/<user>/<repo> (ex: docker.io/dkjer/debian)

$ docker push docker.io/debian
2014/10/01 03:47:52 Error: You cannot push a "root" repository. Please rename your repository to <index>/<user>/<repo> (ex: docker.io/dkjer/debian)

$ docker push dkjer/publicapp
2014/10/01 03:46:47 Error: Missing registry name, try "docker.io/dkjer/publicapp" instead

$ docker push docker.io/dkjer/publicapp
The push refers to a repository [docker.io/dkjer/publicapp] (len: 1)
Sending image list
Pushing repository docker.io/dkjer/publicapp (1 tags)
... [push] ...

$ docker push <IP_ADDRESS>:5000/myapp
The push refers to a repository [<IP_ADDRESS>:5000/myapp] (len: 1)
Sending image list
Pushing repository <IP_ADDRESS>:5000/myapp (1 tags)
... [push] ...

Search behavior
docker daemon not running with --force-fully-qualified=search
$ docker search dkjer
NAME                              DESCRIPTION   STARS     OFFICIAL   AUTOMATED
docker.io/dkjer/publicapp                       0
docker.io/dkjer/otherapp                        0
... [search] ...

docker daemon running with --force-fully-qualified=search
$ docker search dkjer
2014/10/01 03:53:25 Error response from daemon: Missing registry name, try "docker.io/dkjer" instead

$ docker search docker.io/dkjer
NAME                              DESCRIPTION   STARS     OFFICIAL   AUTOMATED
docker.io/dkjer/publicapp                       0
docker.io/dkjer/otherapp                        0
... [search] ...

$ docker search <IP_ADDRESS>:5000/dkjer
NAME                              DESCRIPTION   STARS     OFFICIAL   AUTOMATED
<IP_ADDRESS>:5000/dkjer/mywebserver                0
... [search] ...

Login (register?) behavior
docker daemon not running with --force-fully-qualified=login
$ docker login
Server: docker.io
Username (dkjer):
... [login] ...

$ docker login docker.io
Server: docker.io
Username (dkjer):
... [login] ...

docker daemon running with --force-fully-qualified=login
$ docker login
Error: Missing registry name, try "docker.io" instead

$ docker login docker.io
Server: docker.io
Username (dkjer):
... [login] ...

$ docker login <IP_ADDRESS>:5000
Server: <IP_ADDRESS>:5000
Username (don):
... [login] ...

Local repository behavior
Note: TagStore.Repositories would omit the 'docker.io/' prefix internally to preserve compatibility with existing 'foo/bar' local repository names.  The canonical 'docker.io/foo/bar' form is used for display purposes and should be synonymous with uses of 'foo/bar' alone.
$ docker images
REPOSITORY                        TAG                 IMAGE ID            CREATED             VIRTUAL SIZE
docker.io/debian                  latest              99bedb6181f9        47 hours ago        85.19 MB
docker.io/dkjer/publicapp         latest              c3d5614fecc4        2 days ago          194.9 MB
<IP_ADDRESS>:5000/myapp              latest              b52509553ff3        5 hours ago         1.249 GB
... [images] ...

Has there been any progress on this PR? Following the breadcrumbs of similar PRs and proposals I see a lot of "closing this in favour of that", where "that" turns out the be closed too. This seems like the only proposal about this issue which is still open (but please correct me if I'm missing something obvious).


The following packages have unmet dependencies [xxx] but it is not going to be installed

This happens with any program I am trying to install that has dependencies:
ilyo@DESKTOP-XXX:~$ sudo apt-get install git-core
Reading package lists... Done
Building dependency tree
Reading state information... Done
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:

The following packages have unmet dependencies:
 git-core : Depends: git (> 1:<IP_ADDRESS>) but it is not going to be installed
E: Unable to correct problems, you have held broken packages.

The Ubuntu forums suggest doing this to fix
sudo apt-get update
sudo apt-get upgrade
sudo apt-get install -f

It updated some components but did not solve the problem.
Windows 10.0.14393 Build 14393

Same thing happened to me and I solved it somehow.
What I did was
sudo apt-get install -f
sudo apt-get install dependencies
sudo apt-get install package

So in your case, did you try to install git and then git-core?

Yeah this has only happened to me recently and in the most recent build.  It was very strange.

@shavvn Yes, seems like it can't fins any dependency, git needs some-lib, and some-lib needs another-lib etc.

Is anyone still seeing this issue?


chore(dlp): skip failing test

Updates: #2165

Merging this to unblock CI. Issue is still open to track this failure.


Virtual Machines Managed Identity Info is Missing

Bug Report
Issue: Virtual Machines Managed Identity Info is Missing
Go-sdk version: github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5 v5.7.0
CLI cmd used: az vm list
We had A VM's resources with attached Managed Identity, which should get clientId and principalId in Identity attribute as follow
{
  "id": "/subscriptions/<subscription-id>/resourceGroups/<group>/providers/Microsoft.Compute/virtualMachines/<vm-name>",
  "identity": {
    "type": "UserAssigned",
    "userAssignedIdentities": {
      "/subscriptions/<subscription-id>/resourceGroups/<group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<mi-name>": {
        "clientId": "11111111-1111-1111-1111-111111111111",
        "principalId": "11111111-1111-1111-1111-111111111112"
      }
   }
}

but in our case we are receiving
{
  "id": "/subscriptions/<subscription-id>/resourceGroups/<group>/providers/Microsoft.Compute/virtualMachines/<vm-name>",
  "identity": {
    "type": "UserAssigned",
    "userAssignedIdentities": {
      "/subscriptions/<subscription-id>/resourceGroups/<group>/providers/Microsoft.ManagedIdentity/userAssignedIdentities/<mi-name>": {
        "clientId": null,
        "principalId": null
      }
   }
}

as we can see in the response we are getting the info related to resource id of the Managed Identity but not the clientId and principalId
code snippet to reproduce
listPager := client.NewListAllPager(nil)
for listPager.More() {
  resp, err := listPager.NextPage(ctx)
  if err != nil {
	  panic(err)
  }

  for _, res := range resp.Value {
	  print(res.Name, res.Identity.UserAssignedIdentities)
  }
}

in the res.Identity.UserAssignedIdentities attribute we should see the desired data
CLI output

UI output

repo can obtain clientId and principalId

Hi @lchavan-uptycs, there could be a delay, please try again.


Supplementaries Flax not working

The Flax Crop isnt working with this mod. But since supplementaries is such a common mod Id love you to have a look at it.

Double tall stuff is proving a bit challenging because there isn't a consistent implementation across mods. I'm having the same issue with Immersive Engineering hemp. I'll look into this and see if I can figure out a comprehensive solution. If figured it out for a couple other mods already.


Compile cache for C++ code?

The current method of building code takes a long time.   Maven and Gradle have Maven Build Cache and Gradle Enterprise.   C++ has incredibuild.   Is there some way we can make c++ builds faster through compiler caches?

It looks like ccache is already enabled ccache -s.


Change the title of a bookcase with a bookcase layout

When you have a element that is part of a bookcase layout that has a bookcase layout in itself its not possible to change the title by using # or title tag. It automatically puts the page title as its title and it can not be changed.
It would be nice if it could be changed.

Also, it would be great if you could have a variable that allows you put a custom name on the page on the bookcase layout (https://kaiiiz.github.io/hugo-theme-monochrome/layouts/bookcase/demo/) so that it doesnt use the file name.

When you have a element that is part of a bookcase layout that has a bookcase layout in itself its not possible to change the title by using # or title tag. It automatically puts the page title as its title and it can not be changed.
It would be nice if it could be changed.

It's confusing to use the term "Title" as there are two title-related components on the same page. I'm not sure your title is the blue part or the red part in the above figure.

Also, it would be great if you could have a variable that allows you put a custom name on the page on the bookcase layout (kaiiiz.github.io/hugo-theme-monochrome/layouts/bookcase/demo) so that it doesnt use the file name or page title.

I open a separate issue #32 for this feature.

And the power to put the bookcase layout in between text. Right now, if you write anything in the page the layout is show at the bottom. It would be nice to have an option to be able to pinpoint the location of the layout within the webpage.

This is way more complex than the bookcase implementation right now. I think you need to build such a page with the Blank Layout and a bookcase-like shortcode (currently not implemented yet). This brings you more flexibility and does not clutter the layout options with lots of rarely used options.

Implemented in v2.0.0.


updated to godot 4.0 (alpha1)

~ Edit: wrong PR
I have updated your code to Godot 4 (alpha 1).
I have a problem with the EditorInspectorPlugin. It doesn't make a new entry in the Inspector Panel. Maybe you can help me. (There is no hurry)
All other things seem to be working.
Gamemap

I think it may be better if this PR was targeted for the godot-4.0 branch instead of main. Right now this plugin is used in Godot 3.x and changing the codebase to work for 4.0 might cause confusion. It might be better if we waited for Godot 4.0-stable before making changes to the main branch. Unless if the Asset Library has a way to make different assets for different versions of Godot? I'm not sure how it works.

Sorry, I forgot to set the branch correctly. It should be corrected now.

Hi @Gamemap, nice one!

~ Edit: wrong PR

What does that mean?
Hey, I see lots of cache files from the editor in paths like .godot/..., I think they shouldn't be in git at all. Just add .godot/ to the .gitignore file and remove the files.
I'm also seeing some commits here that are already in main. I'll bump the godot-4.0 branch to the same commit as main, so we can have a clearer look at your changes.
Also, @OverloadedOrama I entered the Asset Library and see some plugins with duplicated entries, one ending with (G4), maybe we could do such a thing for having both versions there. I don't really know either, but might be a solution.

What does that mean?

I had prepared two Pr's for different Godot projects and accidentally copied the wrong text into this Pr.

I am currently on vacation and will continue on Tuesday.

I had prepared two Pr's for different Godot projects and accidentally copied the wrong text into this Pr.

Oh, got it. I think you don't need to explain this kind of edit, don't worry ^^

I am currently on vacation and will continue on Tuesday.

I'm also traveling and will only be able to give this PR greater care next week, I hope you don't mind.

Sadly Github dosn't update the commit view.
Please tell me if I should create a new pr, but for now you can see my changes here:
https://github.com/gilzoide/godot-dockable-container/pull/7/files/c0c631d27178923d28d8631b0116f4efad2cb360..12bf132c1b27bd191c714b9f72f8c68ac21d1ab5

Yeah, that's really weird. It's still showing some old commits, even though I bumped the godot-4.0 branch to where main was.
Feel free to try opening a new PR first to see if it gets the right commits. If it does, we can close this one and continue there.
I still need to download Godot 4.0 to test your patch, but will do =]

Sorry about the delay.
I just tried opening your branch in Godot 4 (to be more specific, I'm using v4.0.alpha2.official [79077e6c1] in a Linux x86_64 machine) and the test scene had an error at TestScene.gd line 19. Apparently BaseButton.pressed got renamed to BaseButton.button_pressed.
Also dragging panels to split the containers did not work.
Were you able to run the test scene and drag/split panels around correctly in your branch?

Apparently BaseButton.pressed got renamed to BaseButton.button_pressed.

Thank you, I will update this.

Also dragging panels to split the containers did not work. The preview of the rectangle appears, but dropping wasn't accepted, so the layout continued the same after releasing the mouse.

I have this problem too. The EditorInspectorPlugin does not make a new entry in the Inspector Panel for the new Label Resource.
I don't know how to fix this. (I didn't understand it with Godot 3.x either) Maybe you can help me with that.

Feel free to try opening a new PR first to see if it gets the right commits. If it does, we can close this one and continue there.

I didn't created a new PR because I had to close this one first to do a new one, and I didn't have enough time to do this last week.

It is working. Thank you very much!
If it would be ok for you I would close this PR to continue with the correct commits in a new one.

Ok, I'll just close the PR then, feel free to open a new one.
I hope it works, not being able to see the latest commits is really annoying.

Superseded by #8.


Prepare 0.14.0 release

@MikeGoldsmith is there any breaking changes we need to highlight about #129 ?


Can't pull images from a private registry

It used to work with 0-3.10-bullseye but started to get this message since this build failed. Not sure if that's related.
docker pull my-private-registry/devcontainers/python:0-3.10-bullseye
WARNING: ⚠ Failed to pull manifest by the resolved digest. This registry does not
	appear to conform to the distribution registry specification; falling back to
	pull by tag.  This fallback is DEPRECATED, and will be removed in a future
	release.  Please contact admins of https://my-private-registry ⚠

0-3.10-bullseye: Pulling from devcontainers/python
manifest verification failed for digest sha256:276b3e6ab551c21e2ba3240ed595a8940194f17cda9742f0e01753f6b65be0f5

However, if changing the tag to 0.204.4-3.10-bullseye, it works. Looks like that tag was created before the build above?
I get above error with tag 0.204.5-3.10-bullseye.
I am seeing this issue with typescript-node images and java as well. e.g it works for javascript-node:0.204.14-18-bullseye but not for javascript-node:0-16
Docker Info
~ ❯ docker --version
Docker version 20.10.22, build 3a2c30b

Hi 👋

It used to work with 0-3.10-bullseye but started to get this message since this build failed. Not sure if that's related.

The build you've linked is a test run failure for jekyll image and, you are trying to pull some python image hence, should not be related.
Looks like things were working for you before releasing these image tags.
@floresjma Have you tried pulling the mcr.microsoft.com/devcontainers/python:0-3.10-bullseye image directly instead of using my-private-registry/devcontainers/python:0-3.10-bullseye from a private registry?
docker pull mcr.microsoft.com/devcontainers/python:0-3.10-bullseye works perfectly for me.

Yes, not issues with pulling docker pull mcr.microsoft.com/devcontainers/python:0-3.10-bullseye. Looks like the problem might be on my private registry side.
Closing this for now, thanks for looking into it.

I did further investigation on my side and the tags I shared before. I was wondering if this could be related that, for example, the Python image with tag 0-3.10-bullseye and 0.204.5-3.10-bullseye are OCI images whereas 0.204.4-3.10-bullseye is not?
For example:
docker buildx imagetools inspect mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye
Name:      mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye
MediaType: application/vnd.oci.image.index.v1+json
Digest:    sha256:cf041df2088399d42393b350c5f78dfadf3242e1fdecaeb9f3937796832b2bdc

Manifests:
  Name:        mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye@sha256:4140139ddd6ea6f0192abb5d2cb07783fdcdb1f2ec9199aa7b1748667eb08729
  MediaType:   application/vnd.oci.image.manifest.v1+json
  Platform:    linux/amd64

  Name:        mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye@sha256:276b3e6ab551c21e2ba3240ed595a8940194f17cda9742f0e01753f6b65be0f5
  MediaType:   application/vnd.oci.image.manifest.v1+json
  Platform:    linux/arm64

  Name:        mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye@sha256:a1b0a7ff4176a17bbbd37c19180ceb794d3fe841e72755417db55b1bf124025b
  MediaType:   application/vnd.oci.image.manifest.v1+json
  Platform:    unknown/unknown
  Annotations:
    vnd.docker.reference.digest: sha256:4140139ddd6ea6f0192abb5d2cb07783fdcdb1f2ec9199aa7b1748667eb08729
    vnd.docker.reference.type:   attestation-manifest

  Name:        mcr.microsoft.com/devcontainers/python:0.204.5-3.10-bullseye@sha256:f4a5be82af4ddefa092c40a7307fdc1b4fa42b1c9ffee7c477ce8d38d63a10b2
  MediaType:   application/vnd.oci.image.manifest.v1+json
  Platform:    unknown/unknown
  Annotations:
    vnd.docker.reference.digest: sha256:276b3e6ab551c21e2ba3240ed595a8940194f17cda9742f0e01753f6b65be0f5
    vnd.docker.reference.type:   attestation-manifest

See MediaType: application/vnd.oci.image.index.v1+json. See that docker buildx imagetools inspect mcr.microsoft.com/devcontainers/python:0.-3.10-bullseye gives me a similar output.
docker buildx imagetools inspect mcr.microsoft.com/devcontainers/python:0.204.4-3.10-bullseye
Name:      mcr.microsoft.com/devcontainers/python:0.204.4-3.10-bullseye
MediaType: application/vnd.docker.distribution.manifest.list.v2+json
Digest:    sha256:b3cadaf960d7f6d464890c53716df4bcc71b176abcb6ab12604e0cc78926a4db

Manifests:
  Name:      mcr.microsoft.com/devcontainers/python:0.204.4-3.10-bullseye@sha256:e5ff28e9e17b8ee174a9103062a23b1734ee72777b424506fb9d5dcabe28c359
  MediaType: application/vnd.docker.distribution.manifest.v2+json
  Platform:  linux/amd64

  Name:      mcr.microsoft.com/devcontainers/python:0.204.4-3.10-bullseye@sha256:280f714e0d88bd13533d1d132d8db5678a75c470b0753d4edac86e80e4c82b41
  MediaType: application/vnd.docker.distribution.manifest.v2+json
  Platform:  linux/arm64

See Name: mcr.microsoft.com/devcontainers/python:0.204.4-3.10-bullseye
Was that change of MediaType to OCI introduced then perhaps? Could that explain why my private registry cannot pull the image?

Was that change of MediaType to OCI introduced then perhaps? Could that explain why my private registry cannot pull the image?

Yep @floresjma, great findings! looks like a change related to support OCI 1.1 by the open containers team - https://toddysm.com/2023/01/05/oci-artifct-manifests-oci-referrers-api-and-their-support-across-registries-part-1/
From https://github.com/opencontainers/image-spec/blob/main/image-index.md, the MediaType is now application/vnd.oci.image.index.v1+json which seems to have recently modified.
Also, the devcontainer CLI was recently modified to support this - https://github.com/devcontainers/cli/pull/393

Thanks for confirming.
Perhaps, it could be good to include somewhere in the docs that the images are OCI format? As some older versions of registries don't support OCI format. Happy to contribute to that doc update if you think that's needed.

Unfortunately, this change wasn't made by us, but by the image registries. MCR/DockerHub/ACR recently updated certain images in the registry to make them OCI-compliant. Looks like a change related to support for OCI 1.1 https://toddysm.com/2023/01/05/oci-artifct-manifests-oci-referrers-api-and-their-support-across-registries-part-1/

Thank you for the detailed response.


Replace uuid4() to generate API keys

Hello,
I was just looking through fastapi security/auth libraries and I saw that in this library, api keys and also the secret value is created with uuid4(). However, UUID is not secure for that purpose and should not be used according to RFC4122. Instead, use something like token_urlsafe from the secrets package.
https://github.com/mrtolkien/fastapi_simple_security/blob/c2db8d70795a2cac682f759296206122df64b5fb/fastapi_simple_security/_sqlite_access.py#L58

You're welcome to make a PR for this.


typo: three_prime_coding_exon [sf#135]

Reported by cmungall on 2008-12-04 05:10 UTC
[Term]
id: SO:0000202
name: three_prime_coding_exon
namespace: sequence
def: "The exon that is most 3-prime on a given transcript." [SO:ma]
synonym: "3' coding exon" RELATED  []
synonym: "three prime coding exon" EXACT  []
is_a: SO:0000147  ! exon
def should presumably say *coding* exon.
Also should the is_a parent of this and it's 5' sibling not be coding_exon?

Commented by eilbeck on 2008-12-04 18:58 UTC
Done.
Thanks
--K

Updated by eilbeck on 2008-12-04 18:58 UTC

status: open --> closed-accepted


Auto refresh does not work in SFX beta

Describe the bug
Looks like the auto refresh is not working properly in the SFX Beta version
To Reproduce
Steps to reproduce the behavior:
Mainly observed while upgrading applications. The refresh indicator shows that a refresh is done but the upgrade state doesn't progress until manually refreshing the web page
Expected behavior
Auto refresh should update the details of the upgrade and the whole SFX details in general
Screenshots

After refreshing manually right after that

Service Fabric version: 8.0.514.9590
OS(for browser not SF): Windows
Browser/ Browser version [e.g. chrome, safari]: Microsoft Edge version 90.0.818.39

@jeffj6123 looks like it is only happening when an application/service is upgrading. The other events seem to be refreshing fine, but only when an application is updating the auto refresh does not seem to be working both in the main page and in the app page itself. Only when clicking the refresh button manually the state gets updated. And it only happens in Beta, when leaving beta it works as expected

@laithhisham going off the screenshots im guessing it was at least 3 minutes between when you visited the page and when you manually refreshed?

@jeffj6123 true, I waited for a few minutes before manually refreshing

so after some investigation it looks like the page level refresh might not get added if there was an error hit during the first refresh attempt.


Batch prover

We ended up not needing combine in the end, and ended up not using those bits of uni-stark code since they didn't support permutation traces


Add a platform setup section and link it in helm install page

Otherwise people will not set up corresponding environment and the installation will fail.
For instance, you need to set up RBAC https://preliminary.istio.io/docs/setup/kubernetes/quick-start/#google-kubernetes-engine in GKE.

[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: wattli
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: sdake
Assign the PR to them by writing /assign @sdake in a comment when ready.
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

content/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

I agree and I can work on it, but not sure everyone like this idea or we can just make a section called 'Platform setup' in that page, I feel the latter is safer. @geeknoid

ya please have @geeknoid weigh in.
Cheers
-steve

Cool, i will ping back when it's done.

@geeknoid , PTAL

Looks good.
@sdake?

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: sdake, wattli
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

content/OWNERS [sdake]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment


Erase flash

This brings in the erase-parts subcommand.  Is that command available upstream?

This brings in the erase-parts subcommand. Is that command available upstream?

Not yet, but I have a PR in to upstream it.  We're leading the charge here.  This uses the long-established EraseRegion command, and figure out the offset and size of the region to erase from the partition table.  It's basically a convenience command.  I probably should add the erase_region subcommand too, for parity with esptool.py.


Implement nodejs:sdk:publish + relevant GitHub Action workflow

closes #3719
related to https://github.com/dagger/dagger/pull/3783#issuecomment-1313687100

Now that https://github.com/dagger/dagger/pull/3459 is in, we can start building this.

Following issue need to be addressed once this PR is merged #3837

Works as expected:


Add testids

Closes #618

Coverage remained the same at 77.219% when pulling 968246b2974ca6507560cc1514bd06f1f997b107 on jdetle:add-testids into 2b508722db834843007970c9c8c9c1d9d930cad5 on gregnb:master.

Totally my bad, will fix that.

Continued here https://github.com/gregnb/mui-datatables/pull/802

Should now be available in 2.7.0.


Errors when executing run_exp_main.sh

After configuring the development environment, I wanted to execute run_exp_main.sh, but got errors:
error[E0061]: this function takes 6 arguments but 5 arguments were supplied
    --> src/optimize.rs:603:41
     |
603  |                     let op = (*g.model).get_or_create_batchnorm(t_inpt, t_scale, t_bias, t_mean, t_var);
     |                                         ^^^^^^^^^^^^^^^^^^^^^^^ ------  -------  ------  ------  ----- supplied 5 arguments
     |                                         |
     |                                         expected 6 arguments
     |
note: associated function defined here
    --> /usr/tensat/target/release/build/tensat-870c490f6fbf864f/out/bindings.rs:8178:27
     |
8178 |             pub unsafe fn get_or_create_batchnorm(
     |                           ^^^^^^^^^^^^^^^^^^^^^^^
8179 |                 &mut self,
     |                 ---------
8180 |                 _input: *const root::taso::Tensor,
     |                 ---------------------------------
8181 |                 _scale: *const root::taso::Tensor,
     |                 ---------------------------------
8182 |                 _bias: *const root::taso::Tensor,
     |                 --------------------------------
8183 |                 _mean: *const root::taso::Tensor,
     |                 --------------------------------
8184 |                 _var: *const root::taso::Tensor,
     |                 -------------------------------
8185 |                 _epsilon: f32,
     |                 -------------

For more information about this error, try `rustc --explain E0061`.
warning: `tensat` (lib) generated 9 warnings
error: could not compile `tensat` due to 2 previous errors; 9 warnings emitted

Is there something wrong? how to solve this problem?

I think this is because you used the original TASO repo. They updated the batchnorm function signature after our work is finished, so it's not compatible anymore. Using the forked TASO version at https://github.com/yycdavid/taso (which is the link in tensat/README) should solve this problem.


Write shorter technical & user manuals, in addition to the technical manual

And maybe reorganize the readme

26286c259e083248ecc879a2d5ecac3eebe9500c


Support for react-lite

I want to decrease the bundle-size by using react-lite.
is there any way, usually I am adding alias  to webpack.

react-lite is not related to React in any way. It may be intended as a drop-in replacement by its authors but we have no guarantees about how well it works, and don’t endorse using it.
If you want to alias react to it, you have to eject (npm run eject) and then you can edit Webpack configs. You can use it without ejecting but only if you import from react-lite itself.
Hope this helps!

I will never eject, thats the magic of this package, simple and clean.
And with each update, I am getting new and better development setup.
I do hope that the package will be a little bit more configurable (native sass support is my most wanted feature)
Thanks for your fast and well explained answer.

You can integrate with Saas by following this section of the docs.

converting a sass to css, on the background, and have duplicates css files is not something I want to do.
I prefer just to forsake sass.

Would be nice if you could overwrite the built in configs for webpack, like adding aliases for (in this instance) react-lite.
For me it works like a charm since I do not want to eject either, but I'm overwriting the alias configs in the node_modules/react-scripts/configs dir. Hardly optimal or recommended but for quick experimentation of bundling with react-lite, it's easily scripted into a build step.


Getting error when restart sync

Hello,I got the log bellow:
[2016/06/17 15:25:15] sync.go:15 [Info] start sync binlog at (mysql-bin.000001, 4969960)
[2016/06/17 15:25:15] status.go:52 [Info] run status http server <IP_ADDRESS>:12800[2016/06/17 15:25:15] sync.go:46 [Info] rotate binlog to (mysql-bin.000001, 4969960)
[2016/06/17 15:25:15] sync.go:50 [Error] handle rows event error ERROR 1146 (42S02): Table 'test.geo' doesn't exist
[2016/06/17 15:25:15] canal.go:146 [Error] canal start sync binlog err: ERROR 1146 (42S02): Table 'test.geo' doesn't exist

I have synchronize my data before, and the index is on a table of a database. Then I insert some data into another table and drop the table after shutdown the sync program. And when I restart the sync program, it got the error. The program found the dump data, and just run according to the binlog. However, the binlog is recorded in order, and the drop record is after the insert record. It causes the problem and the program stopped. The record after the dropping record can't be synchronized.
I think it is important that the synchronization could not be affected by the operations of other table.
At last, I need to delete the dump data before, dump the data, and restart go-mysql-elasticsearch. It worked well.

Hi @blackconcerto
I know this problem, even more, if you alter your table dynamically, it may still fail too.
I have no way to fix it now, for dropped table, we may ignore this syncing data, but for altered table, we can't know the differences easily (you may even change ES mapping too), so I write Can not alter table format at runtime. in readme.


Remote events not triggering on ios

Describe the Bug
Player doesn't respond to remote events in while in background
e.g when i hit on play or pause from backround player
Steps To Reproduce
Run the player while app is in background, and interact and you notice that the remote events not triggered
Code To Reproduce
export const PlaybackService = async function () {

  const remotePlay = async () => {
    console.log('remote play')
    await TrackPlayer.play()
  }

  TrackPlayer.addEventListener(Event.RemotePlay, remotePlay)

  TrackPlayer.addEventListener(Event.RemotePause, async () => await TrackPlayer.pause())

  TrackPlayer.addEventListener(Event.RemoteSeek, event =>
    TrackPlayer.seekTo(event.position),
  )
}

and then in index.js or main app entry file :
AppRegistry.registerComponent(appName, () => App)
TrackPlayer.registerPlaybackService(() => PlaybackService)

the remote events triggered when i'm interacting with the background player - and i validate as the log doesn't output to console.
I also added the capabilities :
const CAPABILITIES: Capability[] = [
  Capability.Pause,
  Capability.Play,
  Capability.SeekTo,
]

export const useVoicePlayer = () => {
  const initPlayer = async () => {
    await TrackPlayer.setupPlayer()
    await TrackPlayer.updateOptions({
      progressUpdateEventInterval: 1,
      capabilities: CAPABILITIES,
      notificationCapabilities: CAPABILITIES,
      compactCapabilities: CAPABILITIES,
    })
  }

Environment Info:
Paste the results of npx react-native info
System:
    OS: macOS 12.0.1
    CPU: (8) x64 Intel(R) Core(TM) i5-8257U CPU @ 1.40GHz
    Memory: 122.63 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.18 - /usr/local/bin/yarn
    npm: 8.5.0 - /usr/local/bin/npm
    Watchman: 4.9.0 - /usr/local/bin/watchman
  Managers:
    CocoaPods: 1.10.2 - /usr/local/bin/pod
  SDKs:
    iOS SDK:
      Platforms: DriverKit 21.4, iOS 15.5, macOS 12.3, tvOS 15.4, watchOS 8.5
    Android SDK: Not Found
  IDEs:
    Android Studio: 2021.1 AI-211.7628.21.2111.8193401
    Xcode: 13.4.1/13F100 - /usr/bin/xcodebuild
  Languages:
    Java: 14.0.1 - /usr/bin/javac
  npmPackages:
    @react-native-community/cli: Not Found
    react: 18.0.0 => 18.0.0
    react-native: 0.69.1 => 0.69.1
    react-native-macos: Not Found
  npmGlobalPackages:
    *react-native*: Not Found

Paste the exact react-native-track-player version you are using
3.1.0
Real device? Or simulator?
real device
What OS are you running?
ios 15.16.1
How I can Help
What can you do to help resolve this?
Have you investigated the underlying JS or Swift/Android code causing this bug?
Can you create a Pull Request with a fix?
I tried to debug and check that i did the setup to use it in background mode as in docs, but i don't know what else i can do

@elirangoshen please read the comment I left you https://github.com/doublesymmetry/react-native-track-player/issues/1657#issuecomment-1240676311

its not working on ios, its not related to android and the nightly build have the same bugs in ios
please reopen
@jspizziri

@elirangoshen can you reproduce this in the example app? I was working with this recently and they were working as I'd expect. I'm happy to re-pen, but I want to rule out a problem with things outside of our library first.

@elirangoshen can you reproduce this in the example app? I was working with this recently and they were working as I'd expect. I'm happy to re-pen, but I want to rule out a problem with things outside of our library first.

Ill try it , yet i want to add that in example app react native version is 0.68.1 and in my app is 0.69.1 (:

That's helpful information. It's possible that it's an issue only in 0.69 which would be important to know.

I see.. is it possible that you might check the example on rn 0.69 ?

We've not yet tested the library yet on rn 0.69. But test on the example with version it has. If it works there then you could upgrade the example to 0.69 and see if the problem starts up.

thanks ! issue was on my side
I was configured notificationCapabilities on updateOptions and it was messing up the background player.
May I know why ?

Interesting, notificationCapabilities aren't even used in iOS so that seems quite strange. Were you able to reproduce in the example?


Migrate more micro-optimisations from Waterfall-Old

Don't attempt to format arguments when there are none provided (-Mystiflow)
Don't create a data input stream for every plugin message we get from servers  (-minecrafter)
Optimize UUID dash replacements (-minecrafter)

Updated with requested changes

Isn't the final keyword for local variables just for source code clearness (if there are no anonymous things inside the method)?

Isn't the final keyword for local variables just for source code clearness (if there are no anonymous things inside the method)?

Yep, just makes sure that no authors in the future do dumb stuff.

@Janmm14 It also frees the JVM to do important optimizations, by assuming the variable remains un-changed after object initialization.

I agree with jamierocks, I'm just going to discard the majority of the UUID PlayerInfo code as I also feel there's too much diff for negligible gain.
I should have the PR updated with other suggested changes in a bit.

Updated to use fully-qualified class imports and fixed mistakes pointed out.

Cheers ;)


Print a better error upon Azure permission errors

This PR catches permissions errors to instruct the user to add specific Azure roles.

looks good to me


Extended custom completions API (alternative approach)

Fixes #1887. This is an alternative approach to #1888 PR.
Instead of introducing extra function, reuse existing one.
With this PR completion causing issue:
_fzf_complete_foobar() {
    _fzf_complete "--multi --ansi -n 2" "$@" < <(
        echo foo
        sleep 10
        echo bar
    )
}

can be transformed to:
_fzf_complete_foobar_func() {
    echo foo
    sleep 10
    echo bar
}
_fzf_complete_foobar() {
    _fzf_complete "--multi --ansi -n 2" "$@"
}

which can be interrupted (ctrl-c) or foo can be selected
Wandering if _func postfix is OK (callback, source ?)
If this approach is OK, I can add bash/fish version.

https://github.com/junegunn/fzf/issues/1887#issuecomment-596057258


Add support for AAD identity connections

Azure Cache for Redis has introduced support for AAD for authentication  and would be great if this was supported by the extension.
This should have a similar configuration experience to the Service Bus extension. For example:
<CONNECTION_NAME_PREFIX>__fullyQualifiedCacheHostName=<CACHE_HOST_NAME>.redis.cache.windows.net

The company I work for are in a position to move one of our services to functions now that there is Redis trigger support but we would much prefer to be using the function's managed identity instead of continuing to handle sensitive connection strings.

Thanks for creating the issue. This is on our radar and we do plan on adding support for AAD authentication for connections.


Add Information Security Corporation (ISC) as Member

@blackbird1999 I will merge this version so that you be listed as a member, please note that you will need to create a description in content/members/infoseccorp.md to also activate the member page itself.


[Draft] Add AI initiative seminar 2024 page

This PR adds a new page for the AI initiative seminar series.

NOTE: this is WIP, please do not merge it.

@KedoKudo just ping me when it's ready. Thanks!

@KedoKudo Thank you for the update. It looks good to me.
@williamfgc can you merge? Thanks!!


Improved Discord Rich Presence

Made this cause I was tired of the discord rich presence always showing up when on desktop
This update adds an option in Visuals and UI asking for what kind of presence do you want (Deactivated, Normal, Hide Infos) and also adds a new define (DISCORD_ALLOWED) so its easier to remove the presence when people make their own mods (Added a queue var when you change the presence while its deactivated so it will resume from that and it also should be usable on lua too)
Also added @NeeEoo's idea: renamed the hx file from Discord to DiscordClient so I could delete a lot of codelines
I added the option in Visuals and UI cause i thought it was the best place where to put, if you think i should change the position just suggest it lmfao
If this gets added, crediting would be cool
Edit: Added a condition when adding the discord_rpc lib into the game
Edit 2: Added the possibility to NOT add the desktop or the DISCORD_ALLOWED check when calling the changePresence function, it wont even get called when no discord (because of inline on the function when no discord and for the fact that the function and class will be empty if no discord)

yay now i can disable discord rpc without editing the project.xml!!!!!
although cuz v0.7 is being worked on right now in a seperate branch, you should just do this in the experimental branch instead

That was actually the idea, but in the end I didnt cause I didnt know if Shadowmario wanted to see already pull requests about that branch

yay now i can disable discord rpc without editing the project.xml!!!!!
although cuz v0.7 is being worked on right now in a seperate branch, you should just do this in the experimental branch instead

That was actually the idea, but in the end I didnt cause I didnt know if Shadowmario wanted to see already pull requests about that branch

I guess in these days then I'll create a new pull request for the experimental branch then

yay now i can disable discord rpc without editing the project.xml!!!!!
although cuz v0.7 is being worked on right now in a seperate branch, you should just do this in the experimental branch instead

That was actually the idea, but in the end I didnt cause I didnt know if Shadowmario wanted to see already pull requests about that branch

I guess in these days I'll create a new pull request for the experimental branch then

alright, suit yourself ig

Very nice idea

Had this idea and added it first in the experimental PR, I suggest to whoever approved this PR to approve the experimental one too (#12225), imma copy and paste the reason of this commit here:
At first I thought it was better leaving them with #if desktop since the discord feature wouldnt be able to do a shit anyway without the DISCORD_ALLOWED, but now that I think about it, its better if I add the discord allowed condition instead of of just desktop, so I reduce the amount of stuff when no discord

Why are you still mainly approving only this one, since the v0.7 is coming out
Yall should check the experimental branch too

Welp at this point, since more and more people are accepting this more than exp version, lemme rebase this to make this experimental, since well, the main branch will begone soon

Failed request. You should probably fix that error

Failed request.
Gets the people going!