Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to fully support ansi colors without ansicon? #461

Closed
badrelmers opened this issue Jun 6, 2023 · 11 comments
Closed

how to fully support ansi colors without ansicon? #461

badrelmers opened this issue Jun 6, 2023 · 11 comments
Labels
external The issue is due to external causes outside of Clink investigation needed Deeper investigation is needed

Comments

@badrelmers
Copy link

Hi, I hope you are fine.

I m on win 7 64, clink does support ansi colors but not this case:

this file contains the ansi colors, in cmd run it by type AnsiColors16_badr.txt
AnsiColors16_badr.txt

with clink I get
image

with ansicon I get the correct output
image

see this second example using miller

with clink I get
image

with ansicon I get the correct output
image

I attempted to run clink and ansicon but it does not work, when I run ansicon after clink nothing happens (ansicon does nothing), when I run ansicon before clink the console crashs as you said here #460 (comment)

can clink support this type of tools that outputs in ansi colors? a lot of tools does this and disabling clink everytime to run those commands is frustrating. I hope there is a solution.

thank you for your time and your excellent work with the new clink.

@chrisant996
Copy link
Owner

Clink is not a general purpose ANSI emulator -- it doesn't affect anything other than Clink output.

type is not Clink output.

Also, other console mode programs are not Clink output.

The crash you experienced may or may not be the same crash as in the issue you linked. (I don't think it's the same crash, I think it's a different crash occurring for different reasons.)

Sometime this week or maybe next week, I'll try to reproduce the ANSICON issues you described on Win7 and see if there's anything Clink can do about them, or if there's a fix I can potentially recommend in the ANSICON repo. Though it seems like development and maintenance stopped on ANSICON a few years ago.

@chrisant996 chrisant996 added investigation needed Deeper investigation is needed external The issue is due to external causes outside of Clink labels Jun 6, 2023
@badrelmers
Copy link
Author

here are the steps I did and it is reproducible:
I downloaded:

https://github.com/chrisant996/clink/releases/download/v1.4.25/clink.1.4.25.e0d48c.zip

https://github.com/adoxa/ansicon/releases/download/v1.89/ansi189-bin.zip

then I open a default cmd with nothing injected in it, then I run:

ansicon -p
clink.bat

then the crash happens immediately, I get this:
image

@chrisant996
Copy link
Owner

That's a crash in Clink (the fault module is clink_dll_x64.dll). That's not a crash in ANSICON, so it is definitely not the same as the crash you cited.

The good news is that it might be possible to do something about a crash in Clink, depending on what's happening and why.

I'll know more when I get a chance (later) to reproduce the issue with a debugger attached.

@chrisant996
Copy link
Owner

Did you intend to launch a separate new console for Clink?

Might Not Be What You Intended

ansicon -p
clink

That injects ansicon into the current console, and then starts a NEW console for Clink ansicon. The crash happens in the new console, and I suspect ansicon actively collides with Clink while they're both trying to hook things. I'll investigate that in the next week or so. Hopefully it will be possible to prevent the crash, but it will depend on specifically what ansicon is doing.

This Is What You Need

First, a one-time action:

clink set terminal.emulation emulate

After that, use the following:

ansicon -p
clink inject

The one-time action tells Clink to use its own built-in terminal emulation. I'll make a change in Clink so that auto for terminal.emulation automatically uses the built-in emulation, and that will simplify things.

And using clink inject injects into the current console, and then it doesn't crash.

chrisant996 added a commit that referenced this issue Jun 7, 2023
ANSI??.DLL (ansicon) has a bad RVA sometimes:
- Run `ansicon -p`
- Run `clink.bat`
- Starts a new console with ansicon injected.
- While Clink is initializing, the first RVA in ANSI??.DLL's import
  table far exceeds the SizeOfImage for the DLL.

Instead of trying to dereference the bad RVA, log it and skip it.
@badrelmers
Copy link
Author

Now it works, thank you so much for guiding me to the right way.

Did you intend to launch a separate new console for Clink?

no, I usually run clink by a shortcut in my start menu,the shortcut call a bat that contains more things and this line:

call "pathtoClink\clink\clink.bat" --profile pathtoClink\clink_profile

so it does not open a new window.

the problem was that I did not add the terminal.emulation = emulate to the clink profile, and also I was not calling clink.bat with inject arg (clink worked without it!), so those two ingredients that you told me now made it work.


I think the doc should be changed for auto in terminal.emulation, it says:

auto = emulate except when running in ConEmu, Windows Terminal, WezTerm, or Windows 10 new console.

and because auto was the default and I m in win 7 and I m not using ConEmu, Windows Terminal, WezTerm, or Windows 10 new console, then I thought that clink already act as in emulate mode, which is wrong as you showed me now, if I m not wrong.


is there a possibility to run ansicon after clink?

this is because ansicon slow down the console, so I do not want it to be running all the time in all my consoles like clink, I want to run it when needed only for one specific console only using a doskey or a bat in the path.

here is a quick test: print 1.3mb txt file with type

22s without clink without ansicon
25s clink without ansicon
44s ansicon without clink
49s clink with ansicon

ansicon made the console take the double of time needed to do the same task.

if it is not possible to load ansicon after clink, can we unload the clink dll?

I attempted to unload the clink dll using process hacker to test if I may be able to unload clink then run ansicon then run clink again, but it failed, the console crashes after unloading the clink dll.

thanks for your time and help.

@chrisant996
Copy link
Owner

chrisant996 commented Jun 7, 2023

the problem was that I did not add the terminal.emulation = emulate to the clink profile, and also I was not calling clink.bat with inject arg (clink worked without it!), so those two ingredients that you told me now made it work.

Users shouldn't have to do that. I've changed Clink so that it automatically uses terminal emulation when ansicon is detected.

I think the doc should be changed for auto in terminal.emulation, it says:

auto = emulate except when running in ConEmu, Windows Terminal, WezTerm, or Windows 10 new console.

and because auto was the default and I m in win 7 and I m not using ConEmu, Windows Terminal, WezTerm, or Windows 10 new console, then I thought that clink already act as in emulate mode, which is wrong as you showed me now, if I m not wrong.

Ah, that's an interesting phrasing quirk. But interestingly, it might be accurate, after the change I committed this evening. 🙂

is there a possibility to run ansicon after clink?

this is because ansicon slow down the console, so I do not want it to be running all the time in all my consoles like clink, I want to run it when needed only for one specific console only using a doskey or a bat in the path.

Ah, I see. Yes, ansicon is known to be very inefficient. For example, it performs a separate console write operation for each individual character, which ruins performance.

Ansicon and Clink conflict when Ansicon is loaded after: Ansicon ends up unable to intercept the output from internal cmd commands like the type command.

if it is not possible to load ansicon after clink, can we unload the clink dll?

Sorry, that's not possible at this time. Un-hooking is inherently risky, and I don't want to invest a ton of time on that and all the support headaches that go along with that.
Clarification: there is no way to make it safe or reliable.

I attempted to unload the clink dll using process hacker to test if I may be able to unload clink then run ansicon then run clink again, but it failed, the console crashes after unloading the clink dll.

Right. You've unloaded the code, but the OS API functions are still hooked and pointing at addresses that you've now unloaded the code from. Crashing is the best you can hope for in that case. 🙂

@badrelmers
Copy link
Author

perfect. the problem is solved , I will wait for the next release to test it (I could not compile it , I got errors).

thank you again for your time, help and explanation, I learned a lot of things.

have a good time.

@chrisant996
Copy link
Owner

UPDATE:

is there a possibility to run ansicon after clink?

No, because ansicon has code here that explicitly refuses to hook an API if the API has already been hooked by anything else. That's why ansicon doesn't affect the type command if ansicon is loaded after Clink -- in that case, ansicon doesn't hook the WriteConsoleW API in the first place (that's the API that cmd.exe uses inside the type command to print the file content to the terminal).

If/when you're able to upgrade from Windows 7 to Windows 8.1 or higher, then you won't need ansicon anymore. In the meantime, there's no way to load ansicon after Clink and there's no way to unload Clink once it's loaded.

(I could not compile it , I got errors).

How did you try to compile it?
What errors did you get?

Maybe you're using mingw, and maybe the mingw build is broken again? Mingw is not a supported compiler for Clink: even when I fix the errors, there are parts of Clink that actually are simply omitted when compiling under Mingw, because of limitations in Mingw.

@badrelmers
Copy link
Author

thank you very much for the time and effort you spent to investigate the ansicon problem. upgrading is difficult if not impossible for me for now, but I solve it by creating two console shortcuts, one for clink that I use daily and another for ansicon to use when needed only.

How did you try to compile it?
What errors did you get?
Maybe you're using mingw

yes, exactly. I used mingw64 x86_64-8.1.0-release-win32-seh-rt_v6-rev0

I got a lot of errors, here is the log
mingw64log.txt

I should have used visual studio after what you told me now.


I downloaded the new version v1.4.26

the crash is solved for the case of:

ansicon -p
clink.bat

but a strange thing is happening now when I colorize the prompt:

  • I unzipped v1.4.26
  • I added this lua prompt script inside clink folder _prompt_by_badr.lua.txt
  • I run clink directly using clink.bat, I got this:
    image

another strange point is that if I run ansicon -p just before starting clink.bat, the colors works fine!!

also, if I use terminal.emulation = emulate the colors works fine too, it seems now that terminal.emulation = auto works like terminal.emulation = native

@chrisant996
Copy link
Owner

Oops, yes, I broke the auto setting on Windows 8.1 and lower. Fixed in v1.4.27. You can use clink update to get the update now, or wait about 3 days for the auto-updater to notice (it waits about 3 days between checking for updates).

@badrelmers
Copy link
Author

that was really quick. it works perfectly. thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external The issue is due to external causes outside of Clink investigation needed Deeper investigation is needed
Projects
None yet
Development

No branches or pull requests

2 participants
-