Terminal: using namespace in header files

[dlong11]

Currently, the repo has a handful of header files that are including "using namespace ...." . This violates Cpp Core Guideline SF.7

• src/host/ft_host/precomp.h
• src/host/globals.h
• src/host/renderData.hpp
• src/host/renderFontDefaults.hpp
• src/host/screenInfo.hpp
• src/host/selection.hpp

If it is decided to fix these, it might be best to tackle these in separate PRs. My guess is removing these will cause lots of other file modifications.

[zadjii-msft]

Summoning @adiviness

[adiviness]

Yup, we shouldn't be doing that. These all seem pretty straightforward to fix.

[MelulekiDube]

Is anyone working on this,

I feel I wanted to take a shot at it.

[DHowett-MSFT]

@MelulekiDube Take a shot at it! Nobody internally has claimed it.

[MelulekiDube]

Thanks, @DHowett-MSFT will get working on it.

[MelulekiDube]

Hello Anyone,

I wanted to check on something. From the readings that I have done, they suggest limiting the using namespace ..... directive into a particular function that wants to use names from that namespace.

I wanted to check before investing a lot of time doing this if this would be the desired solution. Or should I rather just put the using directives on the top of the .cpp files.

I had already started using the latter suggestion where the using directive is limited into a function or some limited scope.

[adiviness]

Using a namespace in a cpp file or in a class declaration or function is fine, but using a namespace in a header is what we want to avoid.

[dlong11]

@MelulekiDube As @adiviness stated - using namespace in a header is should be avoided. SF.7: Don’t write using namespace at global scope in a header file

It is debatable whether the practice of using namespace in cpp files is a "good" practice. The core guidelines rule here is SF.6: Use using namespace directives for transition, for foundation libraries (such as std), or within a local scope (only) The gray area here is what is a "foundation library".

Some C++ projects prohibit the use of using namespace xxxx in cpp files because this makes all names from a namespace available in the file. If you end up adding a lot of "using namespace" in your cpp file you have a greater chance of running into a name collision. And other potential time-bombs.

After looking at some of the windows terminal code - I think some of the use of using namespace in cpp files could be reduced to using . (instead of bringing in the whole namespace)

I also believe that _nested_ C++ namespaces are completely broken are error-prone. Here is a good rationale for this thinking. This is my personal opinion :)

[MelulekiDube]

@dlong11 I think I share the same sentiment with you in that some of the files are using only a specific name from the namespace and in this case, we can reduce using namespace with using that particular name. I will just make everything else follow this and ensure that the changes I had already made are consistent with this.

@dlong11 and @adiviness Thanks for the responses.

[dlong11]

@MelulekiDube This issue is just for using namespace in header files. If you plan to fix using namespace in cpp files, you may want to separate this work and log another issue. This way you can get the terminal team's opinion. My comments about using namespace in cpp files where my opinion and you should verify the terminal team agrees.

This may save you some headaches if the team decides the current practice is OK and the cpp changes are not needed. @adiviness Sound right to you?

[MelulekiDube]

@dlong11 I am not fixing using namespaces in the cpp files I was talking about how to add the removed using namespaces from the headerfiles to the actual cpp file. However, for the most part, I ended up following the style that was there already on the cpp files as most already had using namespace directives.

[adiviness]

Yes, cleaning up the usage of using in the headers is my understanding here. I'm reviewing the associated PR now, thanks for making these changes @MelulekiDube!