microsoft / vscode-makefile-tools

MAKE integration in Visual Studio Code
Other
193 stars 58 forks source link

"Makefile: Build clean the current target" is misleading #136

Closed hitman-codehq closed 3 years ago

hitman-codehq commented 3 years ago

Hello make plugin team!

Plugin version tested: 0.2.0

Can I suggest a renaming of the command "Makefile: Build clean the current target" as this is quite a misleading name. I was searching for how to do a "make clean" and could not find it as I was expecting it to be something like "Makefile: Clean the current target".

There are actually two clean commands:

1) "Makefile: Build clean the current target" 2) "Makefile: Build clean the target ALL"

These behave inconsistently and their naming is a bit misleading. The first one ONLY cleans the current target but the second one cleans and then also builds the ALL target. Given that their naming is almost identical I would expect them to do almost identical things. Also, "build clean" is sort of broken English and doesn't really say what it will actually do. Could I suggest something like:

Makefile: Clean the current target Makefile: Clean the target ALL

Makefile: Clean and then build the current target Makefile: Clean and then build the target ALL

These are then consistent and will behave in exactly the way that their titles suggest, and are much easier to find. Their naming is also consistent with the other two build commands ("Makefile: Build the current target" and "Makefile: Build the target ALL").

Many thanks for the great plugin and I hope my little usability suggestion makes some sense!

andreeis commented 3 years ago

Thank you for the naming suggestions. I understand the grammar behind them. We will think about it. In the meantime, besides the naming suggestions, I want to explain more from our side and to double check with you how these commands work.

You say that "Build clean the current target" only does a "clean", while "Build clean the target ALL" does a "clean" and builds "all". Actually, "Build clean the current target" is supposed to do a "clean" and build the current target (same approach as the other command). Let me know if that doesn't happen for you. Regardless of the naming, that would be a bug we'd like to fix asap. But there's a catch. It also depends on what target you have defined currently. If your current target is "MyTarget", then the command "Build clean the current target" would generate a command like "make clean MyTarget". If your current target is set to "all", it would be identical to the other command. But if you didn't set yet a target, the default is considered empty since also "make" invoked with no target is actually able to identify a default. But I realize now that when you clean the default target, in the end you have only clean: "make clean".

I think we can fix this by invoking 2 make commands: "make clean" and "make", to ensure cleaning and building the default target. What do you think of this approach?

hitman-codehq commented 3 years ago

Hello Andreea.

I finally did some experimentation and yes, I think that invoking with 2 make commands would be better.

For both of the commands they alternate between doing a clean and doing a make. i.e. The first time I invoke "build clean the target ALL" or "build clean the current target" they clean the target, the second time I invoke the command, they build the target, the third time another clean etc.

So the only way to really guarantee the clean + build from one invocation is to explicitly do a "make clean" followed by "make".

Interestingly, for the case where I select "build clean the current target" with no target selected, it only ever does a clean, no matter how many times I invoke it.

So yes, let's take the approach of two calls to make. I have attached my makefile for reference (I had to add a .txt as makefile wasn't accepted).

makefile.txt

andreeis commented 3 years ago

We fixed this issue and until we release 0.2.1 you can try, if you want, a vsix from here (https://github.com/microsoft/vscode-makefile-tools/actions/runs/814645166).

We didn't do any renames yet, only fixed wrong functionality.

hitman-codehq commented 3 years ago

Hello @andreeis, I noticed recently that the make commands had "magically" started to work properly, and then I noticed that this issue had been addressed and rolled out in a new plugin version. I just wanted to say thanks very much! 👍

andreeis commented 3 years ago

Thank you @hitman-codehq for letting us know of this issue in the first place. With your help and the community we will gradually improve this extension. I am glad things work now.

Shadoware commented 1 week ago

Please @andreeis, change the command names as hitman-codehq suggested:

Makefile: Clean and then build the current target Makefile: Clean and then build the target ALL

"Build clean the current target" is very confusing. My birth language is Portuguese and I tried to translate this sentence using two translators but it did not make sense in my language.

hitman-codehq commented 1 week ago

Hi @Shadoware.

I am a native English speaker and it doesn't make sense to me either. The translators could not translate it as it doesn't make sense!