The adapter does not work with karma configuration files not called karma.conf.js

As uncovered in #13.

@cResults Would it be possible to rename karma.conf.src.js to karma.conf.js temporarily. Just to see whether the name is the problem?

Both currently exist.  karma.conf is configured to run against our minimized and compiled JavaScript that is deployed, while karma.conf.src runs against the source files and is great for debugging.
Would changing it to something else be of assistance?

@cResults I am making a test project with a Karma configuration file named karma.conf.src.js as we speak. This should help me track down this error.

I removed "karmaConfigFile": "karma.conf.src.js", from KarmaTestAdapter.json which should run against the default karma.conf and I receive:
[karma] [Projects\Source\Web\ui] Creating KarmaTestContainer for Projects\Source\Web\ui\KarmaTestAdapter.json
[karma] [Projects\Source\Web\ui] Watching Projects\Source\Web\ui\KarmaTestAdapter.json
[karma] [Projects\Source\Web\ui] Watching Projects\Source\Web\ui\KarmaTestAdapter.json
[karma] [Projects\Source\Web\ui] KarmaTestContainer created
------ Discover test started ------
[karma] [Projects\Source\Web\ui] [Karma] [karma]: starting karma server
[karma] [Projects\Source\Web\ui] [Karma] [VS Reporter]: Created
[karma] Refreshing containers: Karma started for Projects\Source\Web\ui
[karma] [Projects\Source\Web\ui] [Karma] [VS Server]: Started - port: 53023
========== Discover test finished: 810 found (0:00:06.7373465) ==========
------ Discover test started ------
[karma] [Projects\Source\Web\ui] [Discover] Discovering tests

It starts discovering tests, but never starts Chrome nor does it finish finding tests.

before that I tried changing the name of karm.conf.src to karm.confsrc.  Change the name of the file and changed the KarmaTestAdapter.KarmaConfigFile.

When I add "KarmaConfigFile": "karma.conf.src.js", back in, I rec'd:
[karma] [Warning] [Projects\Source\Web\ui] Karma configuration file not found: karma.conf.src.js

I think I have resolved the naming issue. The vsix file can be downloaded here:
https://github.com/MortenHoustonLudvigsen/KarmaTestAdapter/releases/download/v1.0.1-issue-15/KarmaTestAdapter.vsix
It has version 1.0.1, so if a version 1.0.1 is already installed in Visual Studio, it needs to be uninstalled before installing this prerelease.

@cResults A more comprehensive log can be found here:
%LOCALAPPDATA%\2PS\KarmaTestAdapter\KarmaTestAdapter.log

I'm getting the "Extension is already installed" error from the vsix

I'm sorry I didn't read your whole message.  Uninstalling and reinstalling 1.0.1

Here's the current output:
[karma] [Projects\Source\Web\ui] Creating KarmaTestContainer for Projects\Source\Web\ui\KarmaTestAdapter.json
[karma] [Projects\Source\Web\ui] Watching Projects\Source\Web\ui\KarmaTestAdapter.json
[karma] [Warning] [Projects\Source\Web\ui] Karma configuration file not found: Projects\Source\Web\ui\karma.conf.src.js
------ Discover test started ------
========== Discover test finished: 810 found (0:00:06.7193014) ==========

Here's the log file.   which exist for every project in our solution.
[13.03.2015 17:09:00.301] KarmaTestContainerDiscoverer created
...
[13.03.2015 17:09:01.094] [Debug] Finding sources for Web
[13.03.2015 17:09:01.226] [Projects\Source\Web\ui] Creating KarmaTestContainer for Projects\Source\Web\ui\KarmaTestAdapter.json
[13.03.2015 17:09:01.564] [Projects\Source\Web\ui] Watching Projects\Source\Web\ui\KarmaTestAdapter.json
[13.03.2015 17:09:01.625] [Warning] [Projects\Source\Web\ui] Karma configuration file not found: Projects\Source\Web\ui\karma.conf.src.js

Right, I've updated the prerelease to log the reason the configuration file is not found - i.e. whether the file does not exist or whether it is not included in the project.

I uninstalled and reinstalled from this link https://github.com/MortenHoustonLudvigsen/KarmaTestAdapter/releases/download/v1.0.1-issue-15/KarmaTestAdapter.vsix
and the output and log look the same as my prior post.
Please let me know if there is something else I can do.
In the meantime, I'm going to trying running tests from a basic non-Require js web project and see if I can get them to run from there.  Then I'll try my paired down project.  If it fails there, I'll upload it for you to use.

@cResults If you are able to reproduce in a paired down project, that would be wonderful! If you can, would it be possible to add it to the TestProjects folder of this project in a pull request?

I have updated the prerelease again:
https://github.com/MortenHoustonLudvigsen/KarmaTestAdapter/releases/tag/v1.0.1-issue-15
Identification of test containers has been improved:

A test container specifies which Karma configuration file to use when running tests.

Test containers are files named karma.conf.js or KarmaTestAdapter.json.

A test container named karma.conf.js specifies itself as the Karma configuration file to use.

A test container, which specifies a Karma configuration file that is not included in a project in the current solution or does not exist, will be disabled. I.e. no tests will be run for the container.

Only test containers, that are included in a project in the current solution, are used.

If there is a KarmaTestAdapter.json file in a project, then any karma.conf.js file in the same directory is not used as a test container.

If there is a KarmaTestAdapter.json in a project in the current solution, that specifies a Karma configuration file in a different directory or project, then that Karma configuration file is not used as a test container.

@cResults Come to think of it, it would be a big help if you would clone this repository (branch issue-15), and see if the adapter works with the test projects in TestProjects/TestProjects.sln.

@cResults I think I have resolved the issues you have raised here. Specifically:

Identification of test containers
Use of globally installed Node modules

So, I have decided to publish version 1.0.1, and close this issue.
However, I am still very interested in hearing the results of your tests, and if possible to see a paired down project. And I will still do my best to resolve further issues you experience.

Hey Morten,
I'm sorry that I was unavailable over the weekend.  I've updated two work stations to 1.0.2 and ran against a project configured the same as the solution I've been working in but only includes smoke tests.
On one it cycles through a loop of restarting karma multiple times and never stops.
OUTPUT:
[karma] [KarmaRequireJasmine/ui] Creating KarmaTestContainer for ui\KarmaTestAdapter.json
[karma] [KarmaRequireJasmine/ui] Watching ui\KarmaTestAdapter.json
[karma] [KarmaRequireJasmine/ui] Watching ui\karma.conf.src.js
[karma] [KarmaRequireJasmine/ui] KarmaTestContainer created
[karma] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[karma] [KarmaRequireJasmine/ui] [Warning] Karma stopped - exit code: 0
[karma] [KarmaRequireJasmine/ui] [Warning] Restarting karma
[karma] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[karma] [KarmaRequireJasmine/ui] [Warning] Karma stopped - exit code: 0
[karma] [KarmaRequireJasmine/ui] [Warning] Restarting karma
[karma] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[karma] [KarmaRequireJasmine/ui] [Warning] Karma stopped - exit code: 0
[karma] [KarmaRequireJasmine/ui] [Warning] Restarting karma

KarmaServer:
[2015-03-16 14:08:43.610] [INFO] karma - starting karma server

KarmaTestAdapter LOG File:
[16.03.2015 14:01:42.907] KarmaTestContainerDiscoverer created
[16.03.2015 14:01:43.414] [Debug] Finding sources for KarmaRequireJasmine
[16.03.2015 14:01:43.428] [KarmaRequireJasmine/ui] Creating KarmaTestContainer for ui\KarmaTestAdapter.json
[16.03.2015 14:01:43.616] [KarmaRequireJasmine/ui] Watching ui\KarmaTestAdapter.json
[16.03.2015 14:01:43.619] [KarmaRequireJasmine/ui] Watching ui\karma.conf.src.js
[16.03.2015 14:01:45.439] [KarmaRequireJasmine/ui] [Debug] [Server] NODE_PATH: e:\Projects\KarmaRequireJasmine\KarmaRequireJasmine\ui\node_modules;C:\Users\David\AppData\Roaming\npm\node_modules
[16.03.2015 14:01:45.448] [KarmaRequireJasmine/ui] [Debug] [Server] Starting Karma: node C:\USERS\DAVID\APPDATA\LOCAL\MICROSOFT\VISUALSTUDIO\12.0\EXTENSIONS\2VSCFXQ3.NAF\lib\Start.js --karma karma.conf.src.js --settings KarmaTestAdapter.json
[16.03.2015 14:01:45.458] [KarmaRequireJasmine/ui] KarmaTestContainer created
[16.03.2015 14:01:46.547] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[16.03.2015 14:01:46.804] [KarmaRequireJasmine/ui] [Warning] Karma stopped - exit code: 0
[16.03.2015 14:01:46.813] [KarmaRequireJasmine/ui] [Warning] Restarting karma
[16.03.2015 14:01:49.163] [KarmaRequireJasmine/ui] [Debug] [Server] NODE_PATH: e:\Projects\KarmaRequireJasmine\KarmaRequireJasmine\ui\node_modules;C:\Users\David\AppData\Roaming\npm\node_modules
[16.03.2015 14:01:49.172] [KarmaRequireJasmine/ui] [Debug] [Server] Starting Karma: node C:\USERS\DAVID\APPDATA\LOCAL\MICROSOFT\VISUALSTUDIO\12.0\EXTENSIONS\2VSCFXQ3.NAF\lib\Start.js --karma karma.conf.src.js --settings KarmaTestAdapter.json
[16.03.2015 14:01:50.116] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[16.03.2015 14:01:50.370] [KarmaRequireJasmine/ui] [Warning] Karma stopped - exit code: 0
[16.03.2015 14:01:50.376] [KarmaRequireJasmine/ui] [Warning] Restarting karma

On the other, the progress bar in Test Explorer continues to cycle and never displays tests.
OUTPUT:
------ Discover test started ------
[karma] [KarmaRequireJasmine/ui] [Discover] Discovering tests

KarmaServer LOG File:
[2015-03-16 14:20:54.028] [INFO] karma - starting karma server
[2015-03-16 14:20:54.243] [INFO] VS Reporter - Created
[2015-03-16 14:20:54.273] [INFO] VS Server - Started - port: 64056

KarmaTestAdapter Log File:
[16.03.2015 14:20:45.627] KarmaTestContainerDiscoverer created
[16.03.2015 14:20:50.592] [Debug] Finding sources for KarmaRequireJasmine
[16.03.2015 14:20:50.604] [KarmaRequireJasmine/ui] Creating KarmaTestContainer for ui\KarmaTestAdapter.json
[16.03.2015 14:20:50.772] [KarmaRequireJasmine/ui] Watching ui\KarmaTestAdapter.json
[16.03.2015 14:20:50.773] [KarmaRequireJasmine/ui] Watching ui\karma.conf.src.js
[16.03.2015 14:20:53.026] [KarmaRequireJasmine/ui] [Debug] [Server] NODE_PATH: d:\Projects\KarmaRequireJasmine\KarmaRequireJasmine\ui\node_modules;C:\Users\dsmith\AppData\Roaming\npm\node_modules
[16.03.2015 14:20:53.029] [KarmaRequireJasmine/ui] [Debug] [Server] Starting Karma: node C:\USERS\DSMITH\APPDATA\LOCAL\MICROSOFT\VISUALSTUDIO\12.0\EXTENSIONS\NGEPDDRC.UYX\lib\Start.js --karma karma.conf.src.js --settings KarmaTestAdapter.json
[16.03.2015 14:20:53.032] [KarmaRequireJasmine/ui] KarmaTestContainer created
[16.03.2015 14:20:54.032] [KarmaRequireJasmine/ui] [Karma] [karma]: starting karma server
[16.03.2015 14:20:54.257] [KarmaRequireJasmine/ui] [Karma] [VS Reporter]: Created
[16.03.2015 14:20:54.280] [KarmaRequireJasmine/ui] [Debug] Karma started using port 64056
[16.03.2015 14:20:54.485] Refreshing containers: Karma started for KarmaRequireJasmine/ui
[16.03.2015 14:20:54.486] [KarmaRequireJasmine/ui] [Karma] [VS Server]: Started - port: 64056
[16.03.2015 14:20:56.937] [Debug] Finding sources for KarmaRequireJasmine
[16.03.2015 14:21:00.257] [KarmaRequireJasmine/ui] [Discover] Discovering tests

Please advise what you would like me to do.

Hey David
Would it be possible for you to share your project with me? It would be great if you could add it to TestProjects/TestProjects.sln and make a pull request. Alternatively you could zip it and send to mlu at 2ps.dk. Hopefully I will be able to reproduce the issue.
/Morten


Webiny serverless cms

Webiny Serverless CMS is a self-hosted, open-source, headless CMS that runs on top of the serverless infrastructure. Scaleable, cost-effective, and innovative solution for managing all of your content needs.

[x] verified that the CMS I'm adding is still maintained.
[x] read CONTRIBUTING.md.
[x] did not generate README.md.

Thank you!


LEGO: Pull request from lego/hb_04604851-bac4-4681-9f74-73de611d6e48_20210705213335944 to main

LEGO: Pull request from lego/hb_04604851-bac4-4681-9f74-73de611d6e48_20210705213335944 to main with localized lcls

Pull Request Test Coverage Report for Build<PHONE_NUMBER>

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 42.754%

Totals

Change from base Build 995411089:
0.0%

Covered Lines:
26168

Relevant Lines:
55576

💛  - Coveralls


Change to Pascal case in OData result with AspNet Core 3.1 is not working as expected

Tried following but didn't work.
services
.AddMvc(options => options.EnableEndpointRouting = false)
.AddJsonOptions(j =>
{
 j.JsonSerializerOptions.PropertyNamingPolicy = null;
 })
.SetCompatibilityVersion(Latest);
Issue re-Created here
Update: Later find out this was due to OData versioning
routeBuilder.MapVersionedODataRoutes("odata", "api", modelBuilder.GetEdmModels());
once the above line change to below
routeBuilder.MapODataServiceRoute("odata", "api", ModelConfiguration.GetEdmModel());
Pascal case works fine. looks like this is something to deal with MapVersionedODataRoutes

Many apologies for the insanely delayed response. I try hard to answer the simple and short answers as quickly as possible. This one slipped by. I must have misread it.
This is a really simple issue. The problem is not MapODataServiceRoute versus  MapVersionedODataRoutes. OData has its own JSON serialization/deserialization. This is why AddJsonOptions has no effect.
The VersionedODataModelBuilder is just a composition of other ODataModelBuilder instances per API version. However, which ODataModelBuilder do you want to use? Who knows? There is a default setting that matches 80/20 rule of what most people want.
Excerpt from here:
public partial class VersionedODataModelBuilder
{
    // Camel case is on by default
    public Func<ODataModelBuilder> ModelBuilderFactory { get; set; } =
        () => new ODataConventionModelBuilder().EnableLowerCamelCase();

    // ...
}

History proved that most people want Camel Casing by default. This is the same behavior as ASP.NET Core itself. If you want Pascal Casing, all you need to do is update the factory property.
For example:
public void Configure( IApplicationBuilder app, VersionedODataModelBuilder modelBuilder )
{
    // EnableLowerCamelCase() is not called; the default behavior is Pascal case
    modelBuilder.ModelBuilderFactory = () => new ODataConventionModelBuilder();
}

That's it! With this simple change, you are off to the races. I hope that still helps.
Thanks

Many thanks @commonsensesoftware taking your valuable time to answer the question, Thanks again!


Promotions Duplicate

Santa Marta Promotion is in Duplicate

http://colombia.experiensa.com/> Promotions > Santa Marta > Win 7 Chrome Version 51.0.2704.84 m also in IE 11 browser> User Public > Santa Marta appears twice in duplicate > Priority High

Just to be clear, even though it is a duplication, it is NOT a bug! This visual issue comes from the fact that we are using the carousel component, which will display images in a carousel style.
Currently the carousel component is configure as follows:

Low screen resolutions (phones) : 1 image
Medium screen resolutions (tablets & laptops): 3 images
High screen resolutions (Laptops & Desktops): 5 images

The duplications comes from the fact that the website had only 4 offers, which viewed from a high resolution screen the component will duplicate 1/4 offerts to be able to be seen in the 5 images carousel
I've added a new offert + I cropped all images so that they are the same size


How to save thumbnails on client-side?

... for one collection. I see example for server-side only.
Just I have library for prepare thumbnails on client-side. 😎

Probably look for a separate client side resizing solution until the cloud upload sub-project is completed (as it's dependant on this feature)


Adding a time series utility.

Also adding a SWEEPER node type.
Will add more comments in line.
Adding a time series utility module for keeping stats. Planning to use it in relay stats. I think it can be more generally usable. Could not find a good existing library. As it is simple enough and also have some unique requirements, just making one. But, if you know of any existing good ones, please let me know and I can change this and make this a wrapper.

i think we might be able to use gonum for this

i think we might be able to use gonum for this

I looked at that @paulwe , but could not find a time series in there. Will check for more options. We can always switch out later too.


applyDiscountCodeChange returns an error for some customers using accelerated checkout

Please list the package(s) involved in the issue, and include the version you are using
"@shopify/app": "3.49.5",
"@shopify/cli": "3.49.5",
"@shopify/ui-extensions": "2023.10.0",
"@shopify/ui-extensions-react": "2023.10.0"

Describe the bug
We got complaints from some customers getting an error when trying to apply a discount with our checkout extension.
Our extension uses the following Shopify mutation to apply discount codes: useApplyDiscountCodeChange.
We get the following error message from Shopify when trying to use useApplyDiscountCodeChange to apply the discount:
"Could not apply mutation applyDiscountCodeChange: the buyer is using an accelerated wallet"
We could not replicate this behavior but we got reports from several customers about this issue.
Expected behavior
We expect to be able to apply discount codes regardless of the customers' use in accelerated checkout.
Screenshots

## Additional context

We recently updated our extension api version to be:
api_version = "2023-10"
Reviewing the 2023-10 documentation for the checkout extension (https://shopify.dev/docs/api/checkout-ui-extensions/2023-10/apis/discounts) reveals the following note:
"This method will return an error if the buyer is using an accelerated checkout method, such as Apple Pay, Google Pay, or Meta Pay."
But this note seems relevant for CheckoutApi - applyDiscountCodeChange and not for useApplyDiscountCodeChange which we use.

Any luck with this? I'm running into the same issue

Hey @edhgoose , this error message is documented below. Is this error message banner being conditionally displayed by your extension? I see this SetErrorState line in your extension code:

We don't recommended displaying this message directly to the buyer since it is not localized (See DiscountCodeChangeResultError on this page)

We've logged being able to Apply Discount Code Changes in accelerated checkouts as a future feature request but this doesn't look like a bug.

Hi @mleandres - I think there's several things in here.
The first is this bug: https://github.com/Shopify/ui-extensions/issues/2133. While we understand that the error message shouldn't be shown, we decided that it was better to show that than nothing.
Now that I've seen a wide range of errors (as documented in the issue), I'd feel more confident in writing a wrapper around your error message that converts it into something friendly for the user that is localised. I just haven't done that yet. Would love your feedback on the other issue as to whether you're likely to add something so we don't need to - as I imagine others would benefit.
The second is:
I understand that you're saying you can't apply discount codes to accelerated payment methods. I assume there's a reason behind this - but for whatever reason it's not supported and that doesn't matter too much. It'd be great if you can tell us why, but I don't see it as being critical.
The thing that's confusing us is that we have no idea how to reproduce this behaviour. As soon as we click to pay with an accelerated payment method, the user is taken out of the Checkout journey into Thank You.
So, how is it possible for a user to be both:
a) In the checkout, and able to interact with our extension
b) Using an accelerated payment method?
We'd love reproduction steps on how to achieve this behaviour, so then we can change our UI. And we'd love guidance on how to detect this circumstance - e.g. should we call applyDiscountCodeChange, and if it fails with this error, should we change the copy of our modal to say something like: "We can't apply your coupon because you're using an accelerated payment method. Please restart your purchase using your coupon code then choose your payment method"?
I don't think we are seeing this issue for anything other than accelerated payment methods - but without a way to reproduce the case, I don't really know 😢

Hey thanks so much for the feedback! We'll definitely take note of that feedback since we're always trying to improve our docs.
As for the accelerated checkout piece, there are some accelerated checkouts that appear "over" our regular checkout such as Google Pay (which appears as a little modal), I think maybe Apple pay does this as well but I'm not sure off the top of my head.
That being said, you should be able to reproduce this with Google Pay, but let us know if you can't get that working.
However, totally agreed that we can do better on documenting error cases.
I'll close this issue but leave the docs suggestions issue open. We do have plans to improve these docs eventually but nothing in the near future.


Reset Feature

What happened?
Hello, while using the ChatInitiator with
class ChatInitiator:
    def __init__(self, user_proxy: autogen.UserProxyAgent, assistant: autogen.AssistantAgent)

If I want to reset the existing context (or in other words make the LLM forget everything that it remembers so far) if a user inputs RESET, how do I do that?
What did you expect to happen?
Reset
How can we reproduce it (as minimally and precisely as possible)?
Hello, while using the ChatInitiator with
class ChatInitiator:
    def __init__(self, user_proxy: autogen.UserProxyAgent, assistant: autogen.AssistantAgent)

If I want to reset the existing context (or in other words make the LLM forget everything that it remembers so far) if a user inputs RESET, how do I do that?
AutoGen version
Latest one
Which package was this bug in
Core
Model used
No response
Python version
No response
Operating system
No response
Any additional info you think would be helpful for fixing this bug
No response

Perhaps check the initiate_chat() function at Line 989 in conversable_agent.py.  The parameter 'clear_history' is by default to beTrue to clear the chat history with the peer agent.

Dear @Kunjal1999
Please see @qgzhang's comment.

Can you please use the markdown code blocks for your code snippet, and the correct version of the package in the future otherwise we might not be able to help you.
Thanks,

Hello,
I want an implementation such that when user inputs "RESET", the agent forgets all history.
class AgentFactory:
    @staticmethod
    def create_assistant_agent(llm_config: Dict) -> autogen.AssistantAgent:
        return autogen.AssistantAgent(
            name="assistant",
            llm_config={
                "cache_seed": 42,
                "config_list": llm_config["config_list"],
                "temperature": 0,
            },
        )

   @staticmethod
    def create_user_proxy_agent() -> autogen.UserProxyAgent:
        return autogen.UserProxyAgent(
            name="user_proxy",
            human_input_mode="TERMINATE",
            max_consecutive_auto_reply=1,
            is_termination_msg=lambda x: x.get("content", "").rstrip().endswith("TERMINATE"),
            code_execution_config={
                "executor": LocalCommandLineCodeExecutor(work_dir="coding"),
            },
        )

class ChatInitiator:
    def __init__(self, user_proxy: autogen.UserProxyAgent, assistant: autogen.AssistantAgent):
        self.user_proxy = user_proxy
        self.assistant = assistant

    def initiate_chat(self, message: str) -> Dict:
        return self.user_proxy.initiate_chat(
            self.assistant,
            message=message,
            summary_method="reflection_with_llm",
        )

Perhaps check the initiate_chat() function at Line 989 in conversable_agent.py. The parameter 'clear_history' is by default to beTrue to clear the chat history with the peer agent.

It does not get cleared, i.e. the LLM still remembers context. I want a functionality such that on pressing RESET the LLM forgets context.

I don't think Autogen maintain a context beyond the conversation history,
unless... you are talking about the cache?
On Thu, 24 Oct 2024, 07:15 Kunjal1999, @.***> wrote:

Perhaps check the initiate_chat() function at Line 989 in
conversable_agent.py. The parameter 'clear_history' is by default to beTrue
to clear the chat history with the peer agent.
It does not get cleared, i.e. the LLM still remembers context. I want a
functionality such that on pressing RESET the LLM forgets context.
—
Reply to this email directly, view it on GitHub
https://github.com/microsoft/autogen/issues/3896#issuecomment-2433476972,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AHCGIBK7XL4H2QMTFZ7ZIDLZ5AGWDAVCNFSM6AAAAABQNT2OQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZTGQ3TMOJXGI
.
You are receiving this because you were mentioned.Message ID:
@.***>

I don't think Autogen maintain a context beyond the conversation history, unless... you are talking about the cache?
…
On Thu, 24 Oct 2024, 07:15 Kunjal1999, @.> wrote: Perhaps check the initiate_chat() function at Line 989 in conversable_agent.py. The parameter 'clear_history' is by default to beTrue to clear the chat history with the peer agent. It does not get cleared, i.e. the LLM still remembers context. I want a functionality such that on pressing RESET the LLM forgets context. — Reply to this email directly, view it on GitHub <#3896 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHCGIBK7XL4H2QMTFZ7ZIDLZ5AGWDAVCNFSM6AAAAABQNT2OQWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZTGQ3TMOJXGI . You are receiving this because you were mentioned.Message ID: @.>

My current application is something like this:
sys_msg = "something"
init_msg = "something"
af = AgentFactory()
assistant = af.create_assistant_agent(llm_config)
user_proxy = af.create_user_proxy_agent()
assistant.update_system_message(sys_msg)
c= ChatInitiator(user_proxy, assistant)
output = c.initiate_chat(init_msg)

Now, whenever a user enters RESET, the LLM should forget everything except for the sys_msg and the init_msg, i.e. it should forget everything that the user sent and it remembered.
E.g.
User: What is the capital of France?
LLM: Paris
User: What is the capital of Germany?
LLM: Berlin
User: Which countries did we talk about previously?
LLM: France and Germany.
User: RESET
LLM: ...
User: Which countries did we talk about previously?
LLM: Did we even talk about countries?

In v0.2, you want to use the register_reply method to add a custom handler that watches out for the RESET keyword and reset the message history.

    @staticmethod
    def create_user_proxy_agent() -> autogen.UserProxyAgent:
        user_proxy = autogen.UserProxyAgent(
            name="user_proxy",
            human_input_mode="TERMINATE",
            max_consecutive_auto_reply=1,
            is_termination_msg=lambda x: x.get("content", "").rstrip().endswith("TERMINATE"),
            code_execution_config={
                "executor": LocalCommandLineCodeExecutor(work_dir="coding"),
            },
        )
        def reset_reply_func(
            recipient: autogen.UserProxyAgent,
            messages: Optional[List[Dict]] = None,
            sender: Optional[autogen.Agent] = None,
            config: Optional[Any] = None,
        ) -> Tuple[bool, Union[str, Dict, None]]:
            if messages and messages[-1].get("content", "").strip().upper() == "RESET":
                recipient._oai_messages[sender] = []
                return True, "Memory has been reset."
            return False, None

        user_proxy.register_reply(
            trigger="user_proxy",
            reply_func=reset_reply_func,
            position=0,  # Highest priority
            remove_other_reply_funcs=False
        )
        return user_proxy

Hello, I tried implementing this but it didn't work out. Let me know.

You need to register it to the other (assistant) agent not the user proxy.

Hello,
I tried registering it to the AssistantAgent and it is still not functioning in the expected way. Could you please show how to capture the RESET input sent by the user?

Can you update the trigger field to the actual instance not the string?
Your code is almost there.

Hello,
Thanks for the suggestion!
The issue with using a register_reply function is that even though we can delete the messages history, it still sends the output once since the reply function is probably called AFTER the assistantagent sends it's reply to the userproxy (after receiving RESET).
What is the difference between chat_messages and _oai_messages?

Also, in a user_proxy, do we have any list of pointers to all the agents the user_proxy has been so far interacting with?
Currently a workaround I see is:
self.reply_at_receive.keys()

@Kunjal1999 Beyond register reply function you also use register_hook and register a custom method to process_all_messages transform the messages into an empty list if RESET is detected in the last message.
You can also use message transform feature to do the same thing, which transform the messages into an empty list if RESET is detected in the last message.
Generally, I can see v0.4 Core API might suite you better if you are looking for more control over how messages are handled. You can take a look at the two agent chat example in v0.4 Core preview: https://microsoft.github.io/autogen/dev/user-guide/core-user-guide/quickstart.html


Mouse middle click pasting doesn't work within the integrated terminal on VSCode Mac

In a Mac terminal window, mouse middle click should paste the selected block of text into the current cursor position. This is not happening in the VS Code integrated terminal:
VSCode Version: Version 1.33.0 (1.33.0)
OS Version: Mac Mojave 10.14.4 (18E226)
Steps to Reproduce:

Launch Integrated Terminal
Drag your mouse to select a block of text from the terminal
Try to use the mouse middle click to paste the selected text
Nothing happens

Does this issue occur when all extensions are disabled?: Yes
Or add option to Integrated terminal Middle Click Behavior similar to Right Click Behavior

This is not a mac feature, some terminals support it but it's not an OS-wide feature like on Linux so it only works within that application/terminal and also doesn't use a selection keyboard. Closing as designed for now.
Note that you can use the default for copy/paste on Windows by tweaking settings https://code.visualstudio.com/docs/editor/integrated-terminal#_right-click-behavior

I would think, the integrated terminal to work as the default terminal in mac. The default Terminal Version 2.9.4 (421.1.1) supports middle click by default. Also adding an option for the user would be ideal, for any user who works on the Mac terminal can work on the integrated terminal with no transition. If not then, is there a way to integrate the mac default terminal properties to VSCode Integrated terminal?

@manojvazirani when https://github.com/Microsoft/vscode/issues/3130 is done you should be able to configure this yourself.


Bad escape sequence when including bootstrap

Thank you for this plugin, exactly what I need for my web component. My component is quite big so that I need to include the bootstrap CSS. That starts with
/*
 * Bootstrap v4.4.1 (https://getbootstrap.com/)
 * Copyright 2011-2019 The Bootstrap Authors
 * Copyright 2011-2019 Twitter, Inc.
 * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
 */

However, I get the following error message:
ERROR in ./src/styles.scss 2:24
Module parse failed: Bad escape sequence in untagged template literal (2:24)
File was processed with these loaders:
 * ./node_modules/sass-to-string/lib/index.js
 * ./node_modules/postcss-loader/src/index.js
 * ./node_modules/sass-loader/lib/loader.js
You may need an additional loader to handle the result of these loaders.
|
>         const styles = `/*
|  * Bootstrap v4.4.1 (https://getbootstrap.com/)

I have been looking for a way to remove comments from (S)CSS automatically, didn't find any, removed the comment manually, got the next error with some other characters it didn't like which are not comments. I'm out of ideas how to fix that. Do you have any ideas what I could try?

Ups, sorry for the title, somehow messed that up. Was supposed to be "Bad escape sequence when including bootstrap"

Hey there, thanks for reaching out! 👋  I think sass-to-string should definitely handle this, but until a fix is released (I'll try to get on it ASAP, but feel free to give it a shot if you feel like it 💪 ), maybe one thing you could try is to minify the SCSS with a loader prior running sass-to-string, but even then you'd probably still hit the same issue.
Will keep you posted!

@Tim-Erwin sass-to-string has a very similar approach in its sandbox (npm start) where it simply loads up bootstrap CSS and appends it to the content of the page as you can see here. Can you maybe provide a repository I could checkout and have a look to help you out?

@Tim-Erwin , thanks for your quick reply.

one thing you could try is to minify the SCSS with a loader prior running sass-to-string
I tried that, but couldn't find a suitable loader. Maybe you can recommend one? Will look again.

Can you maybe provide a repository I could checkout and have a look to help you out?
Should have done that earlier. Turns out, sass-to-string is not the problem, but the fact that I hade node-sass installed. A simple npm uninstall --save node-sass fixed the problem.

Glad you got it sorted out @Tim-Erwin 😄


Add abuse.ch's threatfox integration

We want to regularly query Threatfox, in particular https://threatfox.abuse.ch/browse/tag/c2/ through their API.
If the host respond and the hash is not in top-1M we can then flag it as suspicious, and possibly serve it to an API/store it somewhere.

Working on it through: https://github.com/Hugo-C/threatfox-daily-c2


[linux] Enable STACK_BACK_TRACE on linux

The STACK_BACK_TRACE Macro is disabled on Linux currently since the StackBackTrace class depends on the CaptureStackBackTrace and Symbol resolution Win32 APIs. We should re-enable it since it helps in debugging memory issues and is also used by the Exception object in the runtime.

Just for the record: I did a quick implementation of this using backtrace(3) on Linux. It turns out this is not simple to test because apparently the Windows implementation just records backtraces internally in case the Windows debugger wants to show a previous stack trace of the runtime after a crash. Since to my knowledge gdb does not allow this, and we don't need any extra support to show the native C++ stacktrace for the actual crash in gdb, right now it would seem this just provides a functionality we cannot use so.
The closest thing I could find that might match what happens on Windows is the rr debugger. See: http://rr-project.org/

@digitalinfinity Is this still something we want?


Rename clippy-preview to clippy and stabilize it

related to https://github.com/rust-lang/rust-clippy/issues/3837

r? @alexcrichton
(rust_highfive has picked a reviewer for you, use r? to override)

r? @ehuss

I imagine we will need to add the docs changes that @ehuss mentioned in https://github.com/rust-lang/rust-clippy/issues/3837#issuecomment-523507550

Looks good to me! Want to go ahead and merge the stabilization of cargo fix --clippy here as well? With docs I think this should be good to go

Thanks @yaahc!  This looks good.
For the documentation:
I would just pick a command like cargo-vendor and just duplicate what it does.  The .adoc files are the source, and from there it uses asciidoc to generate the man pages and HTML pages.  There is a Makefile in src/doc/Makefile, with some brief instructions on how to run it.  There's also some instructions in the README.md with how to run mdbook to verify how it looks (we use mdbook 0.3). Some additional notes:

There needs to be a markdown wrapper in src/doc/src/commands to load the HTML.
Don't forget src/doc/src/SUMMARY.md.  The categories are really rough, but I guess it would go in the "build" section, since that is where check lives.
If the makefile touches a bunch of files just to update the date, just revert those files.  The Makefile is not very smart, since it is mtime-based.
I would maybe add a see-also link at the bottom of the cargo-check.adoc page, since it is kinda similar functionality (and conversely from cargo-clippy to cargo-check).

merge the stabilization of cargo fix --clippy here as well?

There was some prior discussion where the clippy team may want to delay the fix command because the suggestions aren't quite ready (the machine-applicable ones have a tendency to fail).  So I think we'll want to delay that part until they're comfortable with it, and it has had more testing.

Oops sorry, I thought the approval here from the clippy folks also covered cargo fix --clippy, but if that's not the case then yeah let's keep them separate

Proposal to merge this PR to make cargo clippy a built-in command.
@rfcbot merge
What is stabilized
This adds a new built-in command named cargo clippy. This replaces the cargo-clippy plugin that ships with the clippy rustup component.
Why?
This makes it easier to provide a better experience using the cargo clippy command by taking advantage of some internal aspects of cargo and making it behave in a more similar fashion to other Cargo commands.
Some specific differences:

Forces clippy to run, avoiding confusing behavior where if you run cargo check and then cargo clippy, Cargo fails to do anything because Cargo believes it is "fresh".
Avoids running the clippy driver on unrelated dependencies.
Makes it easier to integrate with the -Zcache-messages feature.
Command-line help and documentation more closely match the underlying command (cargo check).

Impact
In my belief this will have negligible maintenance burden.
Notes

Not yet implemented: In the future the clippy flags may be promoted to be parsed by Cargo. The -W, -A, -D, and -F flags can control lint levels. Currently they must be passed after a double dash (such as cargo clippy -- -Wclippy::style). There may be a future enhancement that would allow skipping the double dash by having Cargo parse them.
Basically unrelated to this (though made much easier to implement due to the work here), cargo fix --clippy is now available as an unstable feature, and will be stabilized independently. This is currently waiting for the clippy team to be comfortable with the reliability of the "machine-applicable" lints.

References

Initial implementation: https://github.com/rust-lang/cargo/pull/6759
Tracking issue: https://github.com/rust-lang/rust-clippy/issues/3837

@rfcbot concern overall-ux-of-cargo
I've skimmed through the issues tracking the work on cargo/clippy integration this year and I can see that a lot has been done. I think that's great, and I'd like the lints in clippy to be accessible through cargo on stable Rust.
But here's my comment from about a year ago in rust-lang/rust-clippy#1358, all of which I still think today:

I think clippy is a great name for this project and wouldn't suggest changing it, but I'm somewhat concerned about the overall Rust + cargo interface implications of what's happened here.
I know this is implemented as a custom subcommand, but if we start shipping this component by default, we've functionally added a new (official) subcommand to cargo. Is that the intention for devtools in general? - any new devtool with a CLI will just be accessed by a cargo <project-name> interface? Where is the step where we think holistically about the interface our tools present to the user?
If we are going to eventually ship clippy by default, I'd be more comfortable with some sort of unified cargo lint interface, possibly with some way to configure sources of lints, of which clippy would be a default source. Or possibly there shouldn't be a subcommand at all once its shipped by default, and you should instead just say you want these lint passes run in addition as part of cargo build.
I don't care about the interface to the clippy-preview optional subcomponent, but once we start shipping components by default I think we need to revisit how they fit into the overall Rust toolchain experience: given that they will be on equal footing with Rust and cargo at that point, there is both a need for further revision and less constraints on what kind of interface you can have (i.e. at that point you can make PRs to cargo and rustc that are necessary to get the interface you want).

Unfortunately, this post didn't get a lot of engagement from people involved in clippy a year ago. I don't know if there are other threads that are relevant that I should read that would contain more information on this subject that would convince me these concerns should not be blocking on stabilizing the cargo clippy command.
I don't think we need to have a fully configurable linting infrastructure today. But I do think we need to design the interface to be forward compatible with that. I think cargo test provides a good model - though the test harness has not been configurable, the interface is intended to support use with a different test harness someday in the future.
Concretely, I think my concern boils down to two things:

The use of clippy in the interface name. I think this command should be named cargo lint (and the flag to cargo fix should be similarly --lint or something along those lines). I don't think the clippy project should be renamed, but I expect cargo subcommands to be neutral and descriptive, not to contain branding. The meaning of this command would be very opaque to a user who doesn't have pre-existing knowledge of Rust community history.
The pass through of clippy arguments concerns me a bit, because I am concerned it could create a barrier to using other lint sources in the future. I'm not saying I think this is bad per se, I'm saying I'd like to see written up an explanation of how this pass through is forward compatible with other lint providers in the future.

I know that I've been a very absentee member of the cargo team this past year, and I apologize if this has been discussed at length in cargo team meetings I've not been present for. I'll gladly make resolving this concern a priority for me and I hope clippy's lints can be accessible by default to all users of stable Rust very soon.

@withoutboats to be clear, Clippy is already shipped by rustup on stable as cargo clippy, just as an external cargo-clippy subcommand (and has been for a year). This PR changes very little from the immediately-noticeable interface, the CLI is a nicer (and we've opened the door for an even nicer CLI) and a bunch of impossible-to-otherwise-fix bugs are now fixed. cargo fix --clippy is new and being stabilized in #7383, likely after this lands (?), but everything else here existed already, with some bugs. This is largely an implementation detail being "stabilized".
The subcommand already exists with this name, we're replacing it with a cleaner implementation, and cargo clippy-preview was basically a way for us to test the cleaner implementation without stomping on the preexisting cargo clippy binary that has been shipping for a year. This subcommand still needs the clippy rustup component installed to work.
With that in mind, and with stability guarantees, at a bare minimum this PR must add cargo clippy, with a compatible interface. We can discuss adding a separate lint command (and going through some deprecation), but I don't see how that's a blocking concern for this issue.

So onto that topic: I would rather add such a command when it is easy to hook more lints into the compiler. The way clippy does it is not scalable when it comes to tooling (replacing the rustc means that all other tools get confused: a bunch of the bugs this PR fixes arises from that, and this is also why RLS needs to know about clippy).
Furthermore, I think not calling it lint has a lot of benefits: right now you both have all the builtin lints, as well as the clippy lints, and I feel like calling the subcommand lint would be counterproductive and make this distinction more confusing. This is "branding", but it's branding for the set of lints that get loaded, not for the tool itself.
Given that we're trying to deprecate plugins, we're moving in the direction of making custom lints harder to do, not easier. Clippy has discussed supporting a reduced set of custom lints based on a Coccinelle-like syntax. This arises from a bunch of refactorings we're planning internally to make lints easier to write (https://github.com/rust-lang/rust-clippy/pull/3875), though there's a long way to go before any of this can be pluggable. Once we're closer to that it might be worth imagining what a custom lint interface for cargo lint would look like.
Ultimately I don't consider the name to be a significant barrier to learning. I don't think the name lint will have a bunch of people randomly trying cargo lint and being pleasantly surprised it works -- there's no consistent cross-language tool name for this kind of thing (Go uses vet, Python and JS have multiple named linters). People are going to discover this tool through learning materials or cargo's help and I don't think the name matters in either case. I've found "use clippy" to be a popular refrain when folks are talking to newcomers, "use the linter" is less direct.
To some extent, "cargo lint" may actually give off the wrong impression, since in many ecosystems "linter" is really their equivalent of a non-autofixing rustfmt with a couple extra stuff. C++ has a lot of clippy-like tools, and the community usually calls them "static code analysis tools". The main reason clippy uses the word "lint" at all is because that's what they're called in the rustc source.

The meaning of this command would be very opaque to a user who doesn't have pre-existing knowledge of Rust community history

Also, note that the lints are also namespaced under clippy, and are supposed to be namespaced under something. If we were to resolve the opacity you're worried about here, we'd have to rename those as well (to lint::?). I'd argue that if we want support for alternate custom lint sets in the future, giving clippy a neutral name right now would be counterproductive since it would be confusing when we have more than one lint set.
We basically need to keep some name around for the lint set, and if that name exists then we'd need cargo lint --lintsetname, and given that we have precisely one non-builtin lint set right now, with no short-term plans for more, cargo lintsetname seems like a good shortcut to have. And as I explained that a neutral name for the set of lints is counterproductive here, keeping clippy doesn't seem so bad with these constraints.
If we're going to be adding support for custom lint sets in the future, giving the interface a more neutral name now while keeping decent UX seems like a bad idea to me.
It seems like if we were to figure out what custom lint sets would look like, then it might be easier to design this neutral interface, but without that I'm wary of predesigning an interface. There has been very little interest in supporting easy-to-implement custom lint sets from the rustc side, and while clippy may have a viable path for this we're not sure and our primary interest in that direction for now is making clippy's own lints easier to maintain. This is quite a ways away from happening.

The subcommand already exists with this name, we're replacing it with a cleaner implementation, and cargo clippy-preview was basically a way for us to test the cleaner implementation without stomping on the preexisting cargo clippy binary that has been shipping for a year. This subcommand still needs the clippy rustup component installed to work.

It seems like we could keep shipping the cargo-clippy external subcommand if this were shipped as cargo lint and that would not be a breaking change. I do worry about having an apparently slightly buggy UI to clippy live forever, am I correct in remembering that there have been bugs that basically got closed as "wont fix" because they're already fixed by the cargo clippy-preview interface? These bugs would all have to be reopened if we shipped this as cargo lint I expect.

Also, note that the lints are also namespaced under clippy, and are supposed to be namespaced under something. If we were to resolve the opacity you're worried about here, we'd have to rename those as well (to lint::?). I'd argue that if we want support for alternate custom lint sets in the future, giving clippy a neutral name right now would be counterproductive since it would be confusing when we have more than one lint set.

My guess is that the lints would stay name spaced under clippy:: as the lint provider scope, and this would also solve the forward compat issue boats has by having cargo separate the args for providers based on their scope, at least for the -A -D -W args being passed through.

there is both a need for further revision and less constraints on what kind of interface you can have (i.e. at that point you can make PRs to cargo and rustc that are necessary to get the interface you want).

Not sure I'm understanding the point here correctly, but I don't think its a particularly big deal for clippy changes to require PRs to cargo or rustc. Clippy depends on rustc internals so I think thats already a constraint on clippy. And as for cargo's PR process my experience with it has always been easy and positive.

I do see the value to a generic lint interface and I would love to see one, and more linter sources. But I do think I lean in Manish's direction on this one. I think its fine to stabilize this as clippy and then we could immediately open an issue for cargo lint and copy the source of the cargo clippy subcommand and start adjusting it to have the features we would need for a generic lint interface.

I do worry about having an apparently slightly buggy UI to clippy live forever, am I correct in remembering that there have been bugs that basically got closed as "wont fix" because they're already fixed by the cargo clippy-preview interface?

Yes, and it's not just "won't fix", it's also often "can't fix".

My guess is that the lints would stay name spaced under clippy:: as the lint provider scope

Right, which means that people need to learn the name "clippy" anyway.

@withoutboats to be clear, Clippy is already shipped by rustup on stable as cargo clippy, just as an external cargo-clippy subcommand (and has been for a year).

If true, I'm quite unhappy that the concern I expressed a year ago was not engaged with at all and this went through.

Sorry, my previous comment was unproductive. Spoke a bit to @Manishearth just now, and clarified some things.
The division line that I see us crossing here (and that I've been concerned about, including in the comment last year) is when we ship a cargo subcommand pre-installed to everyone by default. If the user has to take special action to get cargo clippy installed as a valid command on their machine (even if that action involves adding rustup components and not cargo install), I'm comfortable with it having whatever interface makes local sense for clippy. But if the interface is a part of the pre-installed cargo interface, I think the interface needs to be examined and supported from the holistic view of the cargo tool.
From that perspective, I have these concerns with the current cargo clippy proposal:

I think using the name clippy in the interface is very inconsistent with other cargo subcommands, which are overwhelmingly short, generic verbs. I think a preferable interface would be to choose a verb for the "run lints" command, and for clippy to be the default provider of lints, just as the current test harness is the default test harness for cargo test. I don't care that it is "lint" specifically, just that it is a short, generic verb like the other cargo subcommands.
I think if we have a command shipped to every machine, that command should work on everyones' machines. I'm concerned that cargo clippy will error unless users install clippy. I know that there are real infrastructural challenges to shipping clippy by default, but I see being able to do that as a prerequisite for making a command to run it available on every single stable user's machine.

I prefer what I think is already the status quo (but I admit I've had difficulty piecing it together from these threads): an external cargo-clippy tool that people can install will call the cargo clippy-preview subcommand, passing whatever bootstrap flags are needed to run unstable features on stable cargo. I think that's a fine compromise in the same sense that we allow std to use nightly features on stable today.

So I think I mentioned this in person already, but every user right now with an up-to-date rustup will have a binary called cargo-clippy on their system (because of how rustup shims work), one which will complain about the missing component if it's missing. Functionally, we're not very far off from it being a built in cargo subcommand.
Admittedly, with these changes calling cargo clippy will complain later in the process than it used to, and we should probably change that anyway.

Putting aside the name lint itself (we could go for vet like Go, or some other synonym, and even lint isn't that bad I just want to be careful about it), I do still have a problem with renaming cargo clippy to cargo foo: You would still need to know the term clippy for the lints themselves.
This leads to a confusing situation where people are still talking about using clippy, but the command is not cargo clippy, it's something else. We can't do away entirely with the name clippy, because that closes a lot of doors for supporting future non-clippy lint plugins, and it also leads to this weird situation where there are basically two builtin sets of "lints" (or whatever we decide to call them), but there's no clear distinction between them.
If we don't get rid of it, you have the cargo foo command requiring knowledge of there being a thing called clippy since you still do #![allow(clippy::whatever)], and the question becomes "what's clippy", to which the answer is "the thing inside cargo foo" which then produces the question "why isn't it called foo" or "why isn't it cargo clippy".
Basically, while cargo foo fixes one inconsistency, it creates another one, gnarlier and trickier to understand.
On the other hand, clippy already has a pretty strong brand within the community, and tying the cargo command to the brand provides an easier learning curve IMO. "Clippy is a thing that gives you extra lint checks. Use it with cargo clippy." That's quite clear and seems to work well.
 The fact that our compiler ships with some default warnings is super important here IMO because as stated earlier if you don't draw a clear distinction there it gets more confusing.
Other languages typically don't do builtin linters, but that's usually worse from a tooling perspective: all the C++ linters are typically really annoying to set up with a build system. (I've often heard the existence of clippy to be a really major selling point from the point of view of folks coming from such ecosystems)

FWIW I do very much want to add a cargo foo for linting, I'm just not convinced we should do it now, I'd rather do it when we figure out a unified lint interface. I'd be down to add a cargo foo --clippy (and keep cargo clippy) though, that interface seems reasonably extensible.

For me, having a cargo command that doesn't work properly by default is a much bigger concern than the name of it, so let's scuttle the discussion of the name for now. The operative question is whether or not we have already crossed the rubicon, as you claim in your first paragraph:

So I think I mentioned this in person already, but every user right now with an up-to-date rustup will have a binary called cargo-clippy on their system (because of how rustup shims work), one which will complain about the missing component if it's missing. Functionally, we're not very far off from it being a built in cargo subcommand.

I just did a fresh install to understand the situation properly, and its basically what I expected. The rustup shim prints that "cargo clippy is not installed" and tells the user how to install it using rustup. I'm very glad that it prints "cargo clippy" and not "clippy" and acts as if there is no "cargo clippy" program installed, rather than acting as if cargo clippy is working fine but erroring because you don't have clippy.
The status quo is very different from how things would be if cargo clippy were a stable part of the cargo interface. "cargo clippy" is not an installed command in any meaningful sense of the word. It's not listed in the cargo book or cargo --help, when you run cargo help clippy or cargo clippy --help, you do not get help info for it until you install cargo clippy. It's just clearly not an installed command on the user machine in any sense except that there happens to be a binary called "cargo-clippy" in /.cargo/bin for unrelated reasons.
Moreover, if the preferred way to access clippy were to ever change (as clearly I would prefer it to), it would be a non-breaking change to change the shim to print that the way to access clippy is to install something else, not cargo-clippy. But if we make this decision, supporting the cargo clippy interface becomes a long term commitment.
I wan to be clear that I am also unhappy with all of these dev tools (meaning also rustfmt and miri) having this faux cargo interface which is totally detached from cargo and not managed as a part of the cargo user experience. I'd be much happier if you had just named these commands clippy and rustfmt and miri. In the future I'd prefer if the dev tools team took that approach when upgrading from cargo installed tools to rustups installed tools.

The status quo is very different from how things would be if cargo clippy were a stable part of the cargo interface. "cargo clippy" is not an installed command in any meaningful sense of the word. It's not listed in the cargo book or cargo --help, when you run cargo help clippy or cargo clippy --help

But we can change this! The goal of the change here was purely to be an implementation detail, I'm totally okay with bringing it closer to the status quo interface-wise. I'm not sure how amenable cargo's codebase is to selectively-visible subcommands, but it should be doable. Like I said, we should probably make cargo clippy error out far earlier than it currently does, that was an oversight.
I'd prefer for it to be a builtin command with good help/book integration, but that's not the primary goal here.

I'd be much happier if you had just named these commands clippy and rustfmt and miri

IMO the benefits of users immediately realizing a tool works like cargo (as opposed to having to learn a new thing) outweighs the downside here.

But we can change this! The goal of the change here was purely to be an implementation detail, I'm totally okay with bringing it closer to the status quo interface-wise.

What is wrong with leaving this interface unstable and having the cargo clippy binary call it using whatever interface hack is necessary to call unstable commands on stable cargo?

I'm ... not opposed to that. @ehuss what are your thoughts?
I'd still prefer if we stabilized it this way for the better help experience (and I'd like the cargo team to discuss this issue when they get the time). But if that can't happen, doing it as a very very thin shim that way is fine as well.

all of these are not "helping users understand that they are like cargo" its "making users think they are part of cargo."

They can be both, I'm saying that the former is important as well

But I just ran git help -a | grep "^\s" | wc (thanks to years of programming experience I now know how to do this) and the git installed on my system has 135 subcommands.  It didn't get that way overnight.

Uh, it kinda did, git comes with both plumbing commands and porcelain commands, it started out with a ton of plumbing commands (really, a bunch of separate shell scripts) and it later got a cleaner (not necessarily clean, just cleaner) porcelain built on top of it. The output of git help is the interface presented to users, git help -a is something you typically use when you're building low level git tools that do new and exciting things (example). You rarely need to know these commands, which is why they're hidden. Cargo seems to be following a similar model, things like generate-lockfile and locate-manifest aren't in the immediate discovery path.

cargo currently has 30. I'm not saying we should never grow the interface, but every addition to it should be held up to a very high standard of quality and consistency with the rest of the experience

I mean, given that it's taken years for clippy to reach the point where it was even important enough to be included in rustup, I don't think it's likely that you'll have a ton of branded commands even in the far future.
I ultimately still don't find cargo clippy to be inconsistent, perhaps because of a different model of looking at things here. You're not just running "lints" (the compiler has those already), you're running the tool known as clippy, which has clippy lints. I strongly feel that the answer to the question "how do I run clippy" should be "cargo clippy", just like the answer to "how do i build my code" is "cargo build". Notably, the question being asked cannot be "how do I run lints on my code" ("cargo lint") because rustc already comes with lints, and that's the inconsistency I'm trying to avoid here. I feel like that would be way more confusing than having to deal with a command with a slightly-different name.
The "brand" name is exposed already in the lint names as mentioned earlier, and at this point it's in many many learning materials. On the other hand, this isn't true of rustc, we just call it "the compiler".

I do think consistency in the arguments is super important -- that's one of the reasons behind this change! cargo clippy now behaves almost exactly like cargo check (previously it tried to but wasn't as good at it). I think this is the high order bit of interface consistency. I'm not convinced the name needs to be consistent for a good user experience, and I still feel like clippy is not inconsistent because of the model I presented above.
I don't see how this contradicts with your experiences either. Cargo making building a rust project easy is kinda the point here, we integrated with cargo because that's the easy way for a user to do anything rusty. The static analysis experience here is miles ahead of any other language because of cargo clippy being easy to use. I don't see a couple commands with different names breaking this experience. We could be more explicit in the help text to make it clear what clippy is ("Checks a project with the clippy static analysis tool to catch common mistakes blah"), but the fact that it has a brand shouldn't be only partially-exposed IMO.

cargo's credibility as a tool depends on the consistency of its experience.

Like manish said, the interface to cargo clippy-preview is essentially identical to that of cargo check, and my understanding is they function very similarly as well. So it seems like the main issue here is the brand itself. I don't have strong opinions on this either way, but if we go with having the cargo-clippy shell out to cargo clippy-preview that needs to include shelling out for the help text as well, because the help text in the new cargo clippy-preview version is MUCH more consistent with the overall cargo experience than that of the old cargo-clippy interface.

If we do end up going with shelling out to cargo clippy-preview instead of promoting it to a regular subcommand does that mean we need to take out the cargo clippy docs I added to the book and man pages as part of this commit? This makes me sad because I personally feel like everybody should be using cargo clippy and I want it to be as discoverable and accessible as possible. That's not necessarily a good reason to base any decision off of but I wanted to mention it.
What I really wish was that we just always shipped clippy and never broke it on nightly and the world was wonderful and perfect :(, but I doubt that's gonna happen any time soon so I see that as a valid reason to not want to bake cargo clippy super tightly into cargo just yet.

This is a plea from an outsider to consider the implications of not merging this. By far the biggest problem with clippy right now is that cargo check; cargo clippy will not produce any output. This is quite frustrating, as it might seem like your code is passing clippy but fails 20m later in CI, and bites absolutely everyone on my team. The best solution I've found is cargo clean -p <project-crates>; cargo clippy, which then costs everyone multiple minutes of unnecessary compilation on the next cargo build or cargo test.
My understanding is that this PR solves that problem neatly, and I've been eagerly awaiting it for months. It's pretty frustrating to see it tied up in a bikeshed over the name of the command.
Personally I don't think cargo clippy is confusing at all. I think @Manishearth is right that Clippy is a brand that everyone in Rust needs to become familiar with if they want aggressive static analysis; at some point you just need to learn that clippy is the Rust static analysis tool. And I agree that putting clippy lints behind some other command, like cargo lint, is only going to make things more confusing when you go to enable/disable some lint and have to figure out why the lint name starts with clippy::.
I get that clippy isn't a verb like most of the other Cargo commands, and that in a hypothetical future world there may be an additional lint provider besides clippy, and having a unified cargo lint or cargo vet command could be useful, but... I think it's a tiny, tiny price to pay for fixing the no-output problem.

The problem is that namespacing these tools under the cargo command, putting them in the cargo book, so on, all of these are not "helping users understand that they are like cargo" its "making users think they are part of cargo." It seems obvious to me that users would think this from the experience you're giving to them.

As a full-time Rust developer who is somewhat uninformed about the management of the Rust project itself, I can tell you I care not at all about the distinction between the Rust language, the Rust compiler, cargo, and clippy. And I prefer it like this! For the most part, these things just work together wonderfully. I don't see why clippy feeling like a built-in part of cargo is a problem at all—in fact, it feels like a huge positive.

This makes me sad because I personally feel like everybody should be using cargo clippy and I want it to be as discoverable and accessible as possible.

Yes, please! It seems to me that clippy has met already met the bar by being distributed as a rustup component—there really aren't many of these—and so concerns about an explosion of additional branded cargo commands are overblown.

I want to thread out the short term goal and long term goal and what I think are the steps to achieving them:
Solving the cargo check problem
To solve the immediate cargo check problem in the short term, I've proposed as an alternative to this PR to have the cargo-clippy command call cargo clippy-preview from stable. Just as we only guarantee that cargo can use the paired version of rustc, we should guarantee that cargo-clippy can only use the paired version of cargo, and this does not lock in the staiblity of the clippy-preview interface.
Making clippy available to all users
I think this is an admirable goal, but I've already listed the constraints from my perspective: a new every-user-facing cargo subcommand should meet at least these requirements:

The subcommand name should be a short, neutral verb (this is not true of cargo clippy).
The subcommand should use a consistent set of argument flags. The normal UX flow should not involve passing -- flags to an underlying tool (unclear to me if this is true of cargo clippy).
The subcommand should always output in a consistent style to other cargo commands (AFAIK this is true of cargo clippy).
The subcommand should work in the way most users need and expect without configuration or additional installation (this is not true of cargo clippy, as clippy is not yet installed as part of the default installation).

Again, the fourth is far more important to me than the first, and involves serious technical and infrastructural challenges, and so I think talking about naming is not productive right now.
I think an alternative for other dev tools projects which wish to be available to all stable users is to be available under their own interface in the default distribution. This would not have an impact on the user experience of cargo, though I'd also like the project as a whole to have a more structured discussion about our UX values around all of our end user facing teams (devtools teams, libs teams, some parts of lang, docs teams, etc).

The subcommand should use a consistent set of argument flags. The normal UX flow should not involve passing -- flags to an underlying tool (unclear to me if this is true of cargo clippy).

Clippy has the same interface as check, but it currently does need -- flags for -Aclippy::foo style arguments. We could change this, we just didn't think it would be necessary.

The subcommand should work in the way most users need and expect without configuration or additional installation (this is not true of cargo clippy, as clippy is not yet installed as part of the default installation).

Would your opinion on this change if this were fixed for stable (and maybe beta)? I think we require working tools for stable already, so it's possible that we could default-install these tools on stable only if other stakeholders are up for it.

I've proposed as an alternative to this PR to have the cargo-clippy command call cargo clippy-preview from stable

@ehuss do you know how we should do this from an implementation standpoint?

Would your opinion on this change if this were fixed for stable (and maybe beta)? I think we require working tools for stable already, so it's possible that we could default-install these tools on stable only if other stakeholders are up for it.

That distinction is a discussion worth having, but I think it needs to happen in a larger way than on this issue. We need to have a clear consensus on what expectations we have on all tools we ship by default.

That's fair.
Anyway, I'll wait for y'all to discuss this and come to consensus within yourselves, and we'll see how to move forward from there.

I think we can have our :cake: and eat it, too. If I understood the suggestions and positions correctly, we have one uncontroversial way forward:

Rename cargo clippy-preview to cargo lint
Make cargo lint unstable so it won't be callable from stable (or call it lint-preview?)
Give cargo lint a --command flag

this will do whatever cargo clippy-preview does right now, but use the argument to --command instead of hardcoded clippy-driver

keep cargo clippy as a subcommand installed by the clippy component, but make it just a shim that calls cargo lint --command clippy-driver

add some hacks so clippy can call the unstable cargo lint even on stable

This will keep the custom name and special cased clippy API entirely in the hands of the clippy project without expanding the stable cargo API

@oli-obk I mean, we don't need to yet introduce cargo lint for this, we can rename cargo clippy-preview to cargo clippy-internal and get the same benefits. This has already been proposed :smile: . I'm okay with it, though I'd prefer if we got the benefits of a full cargo command (a place in help and the docs)
This way we don't have to tie the design of the lint command to this, though I'm definitely interested in kicking off that discussion as well (if a bit skeptical that we can come up with a good design before custom lint engines start existing)

Oh yea, we can definitely start out by just doing that and slowly iterating the unstable design.

This way we don't have to tie the design of the lint command to this, though I'm definitely interested in kicking off that discussion as well

Oh I definitely didn't mean to suggest this to be the final design. I just wanted something that we can reach quickly.

Keeping the command unstable seems to do a good job of not tying down the design and it existing would in theory introduce people to the concept of a future lint subcommand, though i doubt it would be extremely visible, so I imagine most discovery of it would be by word of mouth.

Should not the name of a future lint command just be check? I always just think of clippy as just check but with more lints.

On October 5, 2019 8:47:09 AM GMT+02:00, spunit262<EMAIL_ADDRESS>wrote:

Should not the name of a future lint command just be check? I
always just think of clippy as just check but with more lints.

Agreed. We should fully integrate the two so that check runs both and provides all the output by line/column, rather than all the compiler output followed by all the clippy output.
I don't think we should have a separate cargo lint at all. I do think we should have a separate cargo clippythat just runs clippy, but it can be an alias for an option to check, too.

Agreed. We should fully integrate the two so that check runs both and provides all the output by line/column, rather than all the compiler output followed by all the clippy output.

I think that's already the case? The compiler and clippy lints get printed together. No other compiler output is mixed in because of the way the compiler pipeline works.

Should not the name of a future lint command just be check? I always just think of clippy as just check but with more lints.

slight difference: you want to rerun lints each time you run the command

Sorry I missed this for so long, I have been absolutely drowning in GitHub notifications lately :-(
First off, I think there is a lot of very valid discussion here and given the permanent nature of a change due to back-compat, I think we should not rush this decision.
There are too many points to quote and reply so here is a random selection of thoughts in random order:

lint does have some pedigree - it is used in Java and JS and other places
I think this PR makes perfect sense from the perspective of Clippy's evolution, but I think that @withoutboats is correct that we must consider this from the angle of Cargo's interface.
I think that it is important that Cargo support tools like Clippy and Rustfmt. IMO Cargo is not just a package manager or build system, it is a platform for operating on Rust projects - rustc is one of the tools it uses, but it can also publish to crates.io (and other regitries), which is totally unrelated to compilation, and so forth. Being able to run cargo fmt and cargo fix is a real ergonomic win for Rust, and I would like Cargo to do more of this. Having said that, I don't want Cargo to be a dumping ground, and I think that the line between 'plugin' and 'built-in' is the right place to police that.
I think having built-in commands which are not installed by default is a new category of thing which we should consider a bit more deeply than just adding another Cargo command.
I don't worry too much about back-compat with the existing commands. clippy-preview can be removed - that is the whole point of the -preview suffix. cargo clippy can be supported, but deprecated and removed at the next edition (if we can make things more ergonomic by adding some temporary hacks to Cargo, then that also seems fine, lets be practical about it).
I think the check/clippy issue is in fact wider than that in that cargo build and cargo clippy don't cooperate in any way (cargo check and cargo build should also be better integrated, but that seems like it should be out of scope here).
Even if having other lint providers seems impractical now, I feel that embedding that constraint in the UI is a bad move for the future.

I think that my preferred solution (mentioned by @withoutboats and @spunit262) is that clippy linting should be an argument to check and build, rather than a command of it's own. That has the benefit of integrating nicely with both commands and doesn't require the user to use Cargo twice.
I'm not sure of the exact syntax, but I think something like --lints clippy would be good since it leave the door open to other suppliers of lints some day. We might in fact want something more general like --tool clippy, I'm not sure. Such a flag would not be passed to rustc, it would be handled by Cargo and Cargo would have to know about Clippy and if not installed could direct the user to install it (it is possible to have Cargo but not rustup, so the clippy shim might not always exist). Such a syntax (c.f., --clippy or something like --more-lints) would be extensible in the future but is still not too much typing (we could alias cargo clippy to cargo check --lints clippy either temporarily with a deprecation message or permanently).

I think the check/clippy issue is in fact wider than that in that cargo build and cargo clippy don't cooperate in any way (cargo check and cargo build should also be better integrated, but that seems like it should be out of scope here).

Not sure what you mean by that -- cargo clippy generates rmeta files just like cargo check and i think pipelined builds mean that they help cargo build). Basically, buildandclippyinterop just as much asbuildandcheckdo, because clippy is a modified version ofcheck`

lint does have some pedigree - it is used in Java and JS and other places

This pedigree is somewhat tainted: in some communities it's more on the side of rustfmt things than clippy.

My pushback against starting off with cargo whatever --lints clippy is that we don't know what the design of custom lints in the future will be like, and I'm wary of closing any doors when this thing is very far in the future.
I also want it to be easy to run clippy, CLI flags are not easy. This is a major criticism of Git, for example -- common tasks are often hidden behind CLI flags that you need to memorize (and Git, at least, ships with bash completion!). I think this is a far worse problem than "our verbs are inconsistent", actually. I can't think of any common task that is currently behind a flag in Cargo, it's all "cargo dothething", and that's a very good thing. It's not that I don't use flags, it's just that I typically only use flags in exceptional situations (cross compilation, features, etc).
This is not a problem if we also ship the alias (which can be done through rustup as it's done right now), but we shouldn't deprecate it.
Furthermore, one of the reasons behind this PR is to make cargo clippy work slightly differently from cargo check (which the current implementation is thinly built on): it adds support for passing in -W flags, as well as making it run even when you have a completed build. You'd have to be willing to make these changes to check if you want to roll it into check.

To reclarify my current positions

I would like us to ship a cargo clippy command by default in cargo (but have it complain early if clippy isn't installed)
Failing that, I'm still okay with shipping cargo clippy via rustup as we do today as a shim around some internal hidden cargo clippy-internal command that is enabled with a secret env var or something
I'm also okay with doing the same thing as a shim around cargo whatever --lints clippy, but I don't want to deprecate or remove the one-word way of invoking clippy.
I'm not okay with the recommended way of using clippy being with a CLI flag
I'm wary of designing for a world with multiple lint engines this early given how far off that is.

I can't think of any common task that is currently behind a flag in Cargo, it's all "cargo dothething", and that's a very good thing

I agree with this statement, but come to the opposite conclusion. Although we're used to thinking of Clippy as an action (i.e., a thing to do), I think that is because of history. As a user, I don't actually want to run Clippy, I want to check my code for errors or attempt to build my code, but I want to get Clippy's input as well as the compilers (i.e., the thing I want to do is to check or build my code, and using Clippy is about customising how that thing is done).

Not sure what you mean by that

I mean that I want to try build my program and see Clippy warnings at the same time as the compilers, i.e., I want a single command that does cargo clippy && cargo build (but integrating the warnings together).

:umbrella: The latest upstream changes (presumably #7450) made this pull request unmergeable. Please resolve the merge conflicts.

As a user I strongly agree with the sentiment that Clippy lints should just integrate into the normal build process. My preference would even be to note in the Cargo.toml that Clippy should be active for this crate, and have the lints run during all kinds of builds (these is the issue I assumed @nrc meant about integrating cargo clippy and cargo build better, they might share output files but they don't share output warnings). This would allow me as a project maintainer to not have to tell contributors anything about running Clippy, they would just run cargo test as normal and either see Clippy warnings output, or a warning like this project has registered the `clippy` lint provider, but you do not have it installed, you can get it by running `rustup component add clippy` .
I believe having lint providers configured in the Cargo.toml would also help integrating with other ecosystem tools, e.g. RLS currently requires you to explicitly add #![warn(clippy::all)] in order for it to detect that it should be using Clippy when producing warnings.

There has historically been a lot of pushback from other teams to running
Clippy as part of the standard build process. That option isn't being put
on the table here.
On Wed, Oct 16, 2019, 2:27 AM Nemo157<EMAIL_ADDRESS>wrote:

As a user I strongly agree with the sentiment that Clippy lints should
just integrate into the normal build process. My preference would even be
to note in the Cargo.toml that Clippy should be active for this crate,
and have the lints run during all kinds of builds (these is the issue I
assumed @nrc https://github.com/nrc meant about integrating cargo clippy
and cargo build better, they might share output files but they don't
share output warnings). This would allow me as a project maintainer to
not have to tell contributors anything about running Clippy, they would
just run cargo test as normal and either see Clippy warnings output, or a
warning like this project has registered the clippy lint provider, but
you do not have it installed, you can get it by running rustup component add clippy .
I believe having lint providers configured in the Cargo.toml would also
help integrating with other ecosystem tools, e.g. RLS currently requires
you to explicitly add #![warn(clippy::all)] in order for it to detect
that it should be using Clippy when producing warnings.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rust-lang/cargo/pull/7382?email_source=notifications&email_token=AAMK6SHWSURAIQHX5EORUXLQO3NBTA5CNFSM4IYDTLEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBL2AAA#issuecomment-542613504,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAMK6SEOVZE3KIK2WIXNHB3QO3NBTANCNFSM4IYDTLEA
.

The Cargo team discussed the issues brought up here, and we'd like to propose the following alternative:
Given the primary motivator here is to fix the fingerprint/caching issue, we'd like to extend Cargo in such a way that the cargo-clippy plugin can work properly, and remove the proposed clippy-preview command.
Background: Currently cargo-clippy uses the RUSTC_WRAPPER environment variable to replace rustc with clippy-driver, which uses rustc_driver to extend rustc. It then calls cargo check to perform linting. Currently, RUSTC_WRAPPER is not included in the fingerprint, so Cargo thinks the result of cargo check is identical to cargo clippy and does not rebuild.
The proposal is to add a RUSTC_WORKSPACE_WRAPPER environment variable. This variable would be used for workspace members only, and take precedence over RUSTC_WRAPPER. This variable will be added to the fingerprint, so that clippy results will be independent from cargo check results.
@yaahc indicated they'd be willing to implement this, which will initially be only used behind the -Zunstable-options flag. This will give use the opportunity to test it out with the cargo-clippy plugin. We hope to stabilize it relatively soon afterwards.
Some of the other issues with cargo-clippy can possibly be addressed by other means, and should be done independently as desired. For example, one of the concerns is that the --help page is incomplete. It may be possible for cargo-clippy to run cargo check --help and re-display that output (possibly with some modifications), and then append its own additions. Or it could bake-in a near-copy, edited as desired. This isn't as clean, but is a problem with any cargo plugin, that perhaps some future work can make smoother.
This maintains the status quo of the current solution while providing a fix for the immediate concern. If there is a desire in the future to have a more extensible or integrated linting experience (however you want to define that), and someone willing to drive it, then that can be a separate project for the future, but I don't foresee that happening soon.
How does this plan sound to everyone? Were there any other motivators that would cause issues with this arrangement?
(Note: I have a separate comment on #7383 for integrating with cargo fix.)

This plan gets a big +1 from me.
Also I'd like to point out for everyone else that the internal variable in cargo that is being set via RUSTC_WORKSPACE_WRAPPER already exists and has been merged. It and was added to make it possible to implement both cargo clippy-preview and cargo fix --clippy. The new plan is to just expose this so that all the necessary changes to make cargo clippy and cargo clippy --fix work the way we want can be done outside of cargo.

@ehuss i'm concerned about including it in the fingerprint: won't this mean that cargo clippy will delete existing metadata from a previous check run? i'd rather clippy not cause problems with other workflows, it should be able to read and write the same metadata
Otherwise this sounds great.
I'm not a huge fan of the wrapper model -- i feel like PRIMARY_RUSTC would be better than PRIMARY_RUSTC_WRAPPER -- but I'd defer to y'all for which plugin interface you want to encourage.

Oh, I should have been clearer on that.  It should include it in the metadata hash, not the fingerprint (it doesn't really matter if it is in the fingerprint, it's probably not necessary).  That way, the clippy artifacts will have a separate filename, and won't collide with check.
As for RUSTC vs RUSTC_WRAPPER, I think clippy is using the wrapper variant intentionally (https://github.com/rust-lang/rust-clippy/pull/2087).  I'd be reluctant to switch that, but if there is some way to make it work, that's an option.
I just realized in my note on fix --clippy that I mentioned using RUSTC, but clippy expects RUSTC_WRAPPER.  That will need to be considered, so maybe I should have said RUSTC_WRAPPER there, and the code will need to inject the extra arg as needed.

oh, yeah build scripts do make this an issue. Let's stick to the wrapper then.

That way, the clippy artifacts will have a separate filename, and won't collide with check.

Hmm, ideally they should be able to share artefacts since clippy doesn't do anything to change the generated artefact, but i guess this is fine

Hmm, ideally they should be able to share artefacts since clippy doesn't do anything to change the generated artefact, but i guess this is fine

Yea, there's not much that can be done about that for now.  The only consequence is a little wasted disk space.  The dependencies will all be shared, just the workspace members might have two copies.  I personally would also be fine with just putting it in the fingerprint if the extra disk space usage is a concern, I don't really care either way.

Confused by the discussion around WRAPPER. My understanding is that clippy currently sets primary_unit_rustc which does not interact with the wrapper itself. The compile step will pick between invoking rustc_process and primary_unit_rustc_process depending on if its a workspace member or not. My understanding is that this should be independent from the wrapper, in that both will be executed with the same wrapper.
I'm not really sure if this has any impact on the discussion around fingerprints and metadata hashes but I want to make sure I'm not misunderstanding how people want this implemented.

Confused by the discussion around WRAPPER. My understanding is that clippy currently sets primary_unit_rustc which does not interact with the wrapper itself.

clippy-preview does that, but the proposal is to remove it.  The current cargo-clippy plugin would need to be changed to set a new environment variable (RUSTC_WORKSPACE_WRAPPER). Cargo would need to read that, somewhere.  I haven't thought about how best to structure that.  It could go in Compilation::new, or Compilation::rustc_process, or in the Rustc struct itself.  Compilation::rustc_process would be the place where it makes the decision on which process to use.  Then either compute_metadata or the fingerprint will need to hash the value of that environment variable.


File Upload App added

Description 📜
Fixes #5

Type of change 📝

[x] Add a new project

Domain of Contribution 📊

[x] Project using only HTML & CSS

Checklist ✅

[x] I have performed a self-review of my own code or work.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] My changes generates no new warnings.
[x] I'm GSSoC'22 contributor

Screenshots / Gif (Optional) 📸

hey @khushi-purwar. I have added the FIle upload app. Please review it and merge it.


[BACKLOG-16818] - Adding data-bricks depdency for avro output execution

@tkafalas please review

this pr is related https://github.com/pentaho/pentaho-karaf-assembly/pull/376 and needs to be merged together

Build Skipped
:white_check_mark: There was nothing to build. The changes are probably excluded by the build rules. If this is an error, please contact your system administrator.


Metrics for Dynamo capacity

Part of https://github.com/weaveworks/monitoring/issues/12

This change is

Parametrized polling interval (and fixed a lint warning while I was there).
Leaving metrics as-is.


Cannot transpile folder starting with dot

I filed this issue a year or two ago:
https://github.com/Microsoft/TypeScript/issues/13399
Unfortunately stuck with a similar problem in a different context, here is the context:
https://stackoverflow.com/questions/53678214/transpile-folders-starting-with-dot
even if I change the include array to be:
  "include":  [
    "src/**/*.ts"
  ]

it still doesn't capture the .test dir

@MartinJohns it doesn't work sadly. what happens is I get folders outside of the src dir, being put into the dist dir. Do you follow?

probably the case of needing "rootDir" option in tsconfig.json


Load Astarte Interfaces From a Canonical Path

Edgehog Device Manager should load interfaces from a "canonical installation path". Such as when installed to /usr/bin/ it should look for interfaces on /usr/share/edgehog-device-runtime/astarte-interfaces/ (or even /usr/share/edgehog/astarte-interfaces/).
When installation prefix is changed, it shoud behave accordingly (e.g. /usr/local/share/edgehog-device-runtime/astarte-interfaces/` etc...)

Or, it could be add in the configuration file of Edgehog Device Runtime :

astarte_interface_directory = "/usr/share/edgehog/astarte-interfaces/"


Font rendering with glium_text is really ugly

Maybe we could use another library like freetype.
I don't know how to implement the change though.

I'll collect some resources on the topic in this thread if its okay. I'll also start working on an implementation but I can't promise a PR in the near future ;)
There is this paper from Valve on a improved method using textured quads for each character:
http://www.valvesoftware.com/publications/2007/SIGGRAPH2007_AlphaTestedMagnification.pdf
And heres some StackOverflow on the topic:
http://stackoverflow.com/questions/5262951/what-is-state-of-the-art-for-text-rendering-in-opengl-as-of-version-4-1
http://stackoverflow.com/questions/2071621/how-to-do-opengl-live-text-rendering-for-a-gui
And the OpenGL Wikibook on this topic: https://en.wikibooks.org/wiki/OpenGL_Programming/Modern_OpenGL_Tutorial_Text_Rendering_01

For longer lines, the rendering degrades. Here's screenshots for two files.
seq -s ' ' 1 1 10 > short.txt
seq -s ' ' 1 1 100000 > long.txt

Loading the file with one long line wreaks havoc on my graphics card, probably due to allocating huge bitmap.


Autorest Regen Preview to 3.0.0-alpha.20231227.2 by Rodge Fu from refs/pull/4062/head

Triggered from https://github.com/Azure/autorest.csharp/pull/4062

API change check
API changes are not detected in this pull request.


Live share opens wrong window

I was looking at troubleshooting why VS Code Live Share was not working for me and my team. The problem we have is that after we've logged in, either trough Microsoft or Github, you are prompted in the browser to open VS Code. The problem is that it opens a new VS Code and you are not connected to the Live Share session.
We are all using Linux / Debian.
Any ideas?

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

ID: 62d2dbef-9f65-dfc7-3d5d-b74b7d88c399
Version Independent ID: 09d0d40c-987f-112a-de83-8a3db354b2b9
Content: Troubleshooting - Visual Studio Live Share - Visual Studio Live Share
Content Source: docs/troubleshooting.md
Product: visual-studio-family
Technology: live-share
GitHub Login: @Chuxel
Microsoft Alias: clantz

As a workaround, can you try running this command: Live Share: Disable VSCode account authentication? This will revert the authentication to the old flow. Please let us know if this unblocks you.

 Initial testing between me and 1 colleague worked great. I hope it keeps working once we do some mob programming with a bigger group of people. We're usually 4 or 5.
Maybe this little "last resort" trick should be in the documentation?

Update, yesterday we managed to have mob programming with 4 developers using this little trick. Thanks again!


syncback: fix namespace escaping

Fixes #209
Problem
The shellcheck PR quoted $namespace in syncback's krsync.sh. It turns out $namespace is not a namespace but either '' or '-n some_namespace'. Quoting the latter breaks the script, yielding this error:
error: pod, type/name or --filename must be specified

Solution
Use the syntax described in the exceptions section for SC2086.
I skimmed the shellcheck PR and didn't see any other instances of this issue.

will investigate what it would take to test for this