tonerdo / readline

A Pure C# GNU-Readline like library for .NET/.NET Core
MIT License
810 stars 77 forks source link

Should not use 'System' as the namespace to qualify your library #58

Closed goblinfactory closed 4 years ago

goblinfactory commented 4 years ago

Readline should not use System namespace. This will make it not possible to use this library in any enterprise project that has any package reviewing process.

I suggest the root namespace be created called something e.g. Tonerdo or similar and a new release be created (version 3) with the new namespace.

Great library btw! nice :D

Latency commented 4 years ago

The default namespace is 'ReadLine'.

namespace ReadLine {
...
}

Use my latest release if you are having issues. Tonerdo has been busy to maintain this repo and merge the PRs associated.

Where are you seeing 'System' namespace being used?

Removing references to 'System' namespace is not going to happen. Console.Read() is found within the 'System' namespace.

Using Win32 to pinvoke and proxy the STDIN as described here, is not necessary. This API is going to remain as is since it works just fine according to feedback from our user base.

I think we have done a fairly good job recreating this API for Windows using pure managed libraries.

goblinfactory commented 4 years ago

re: where are you seeing System

Screen Shot 2020-01-20 at 11 14 16

your source file -> ReadLine.cs

re: Using Win32 to pinvoke and proxy This issue doesn't mention that.

goblinfactory commented 4 years ago

re: Removing referneces to System namespace is not going to happen Ah, I see the confusion. I have renamed the issue title

old title :" Should not use System namespace "

new title : "Should not use 'System' as the namespace to qualify your library"

Re: I think we have done a fairly good job I agree, which is why I care enough to give helpful feedback. Also why I wrote, "Great library btw! nice :D" at the bottom of the issue. grin! keep it up

Latency commented 4 years ago

Do not use Tonerdo build or old release.

The 'System' namespace was refactored a long time ago.

Use my forked release found on my page. It is the latest release since I do not have admin access to merge into 'master'.


This issue should be noted as resolved / closed!!!

goblinfactory commented 4 years ago

just checked it out! txs looks good 👍 link here in case anyone else reads this thread later : https://github.com/Latency/ReadLine

cheers, A