Use compilation-filter as subprocess filter

[BlankSpruce]

No description provided.

[psibi]

Ah, I didn't realize that this was only available for 28.1. I guess we would need some sort of conditional then with xterm-color fixing the actual bug too...

[BlankSpruce]

If you consider using compilation-filter then I don't know how to approach that. On one hand I understand that you would rather preserve already existing functionality but on the other hand this package shouldn't add its own functions to compilation-filter-hook because it'll affect things outside of the package (like M-x compile). And if the package doesn't register such function itself then the user needs to discover it themselves which brings us to back to square one.

Another way would be offering customization point on the filter with the suggestion about compilation-filter. What do you think?

[BlankSpruce]

Just to clarify the point on 28.1: as in the note ansi-color provides means to colorize compilation buffers but before 28.1 there was no convenient function definition so you had to define this small function yourself. I'd say it's not a bad trade-off especially when the README and doc comment provides example how to do that.

[psibi]

Emacs has the variable emacs-version. We need to use that and switch to appropriate version of compilation-filter-hook based on that to support both the versions.

Let me know if you have more questions. Alternatively if you want me to work on top of your work, let me know and I can take that up.

[BlankSpruce]

Yes, I'm aware of emacs-version but that's not the main point of my observation. The thing is I'm of the opinion that this package should never, on its own, register its own functions to compilation-filter-hook because this customization point is outside of the scope of the package. We can use compilation-filter to have unified behavior between compile and justl-exec-recipe but customization of that filter should be on the user side.

If you'd rather have this package automatically register function to colorize the output then I'd opt into either of these solutions to mimic the behaviour of compilation-filter:

• providing customization point on process filter (named like justl-output-filter)
• providing hook on predefined filter (named something like justl-output-filter-hook)

Since you're maintainer it's up to you to decide and I'll try to implement that.

[psibi]

Is there a way to handle both - the colors as well as proper carriage return handling ?

I just checked eshell and it seems to be doing both jobs fine.

[BlankSpruce]

I've pushed another approach. Let me know what you think about that.

I've tested it with justfile:

foo:
    #!/bin/bash
    printf "1/5\r"
    sleep 1
    printf "2/5\r"
    sleep 1
    printf "3/5\r"
    sleep 1
    printf "4/5\r"
    sleep 1
    printf "5/5\n"
    sleep 1
    printf "DONE\n"

bar:
    #!/bin/bash
    printf "\x1B[0mThis is normal text\n"
    printf "\x1B[31mThis is red text\n"
    printf "\x1B[32mThis is green text\n"
    printf "\x1B[34mThis is blue text\n"
    printf "\x1B[0mThis is normal text\n"

foo target:

bar target:

[psibi]

Nice work! This looks good to me. Can you see why the tests are failing ?

I don't mind disabling it for 26.3 if we aren't able to make things work.

Also, do you mind adding a test for the carriage return ? (If you want do that as part of separate PR, it's fine and I can merge this once the CI passes green)

[psibi]

Do we need this anymore ? Since your implementation now handles both, I'm assuming it's not required anymore.

[BlankSpruce]

I'd say it's nice to have customization point. For instance it's not impossible to use compilation-filter as I originally showcased and someone might have some specific hook for compilation-filter.

[psibi]

I would want to avoid customization unless it's really required.

For instance it's not impossible to use compilation-filter as I originally showcased and someone might have some specific hook for compilation-filter.

In the context of justfiles and it's execution, do you see any reason why someone might want to provide their own hook ?

[BlankSpruce]

Except for the point above I don't have explicit example. I'll remove the customization point then.

[psibi]

Ditto for this too.

[BlankSpruce]

Yes, I'll prepare the test and check 26.3 regression later today. I just wanted to discuss this proof of concept before doing that work.

[psibi]

Perfect! Nice work on the test. I'm merging this PR.

Can you do this in a follow up PR: Update the changelog and also add test for a just target involving color (the one you have shown in the comment) ? If not, I can probably add that up in the coming days.