[riot-notifications] [RIOT-OS/RIOT] makefiles/color: detect if writing to terminal. (#12127)

Francisco notifications at github.com
Fri Sep 6 14:40:54 CEST 2019


fjmolinas approved this pull request.

At first I didn't understand the testing procedure that had been done in the original PR (if it was correct), and honestly I didn't now what `at` was, I though I was just overriding the default `TERM` (I now read about it [here](https://www.computerhope.com/unix/uat.htm)).

<details><summary>at now + 2 minute</summary>

```
warning: commands will be executed using /bin/sh
at> make -C examples/hello-world
at> <EOT>
job 11 at Fri Sep  6 13:51:00 2019
```

```
Return-Path: <francisco at francisco-MacBookPro>
X-Original-To: francisco
Delivered-To: francisco at francisco-MacBookPro
Received: by francisco-MacBookPro (Postfix, from userid 1000)
	id D39C94218B7; Fri,  6 Sep 2019 13:51:02 +0200 (CEST)
Subject: Output from your job       11
To: francisco at francisco-MacBookPro
Message-Id: <20190906115102.D39C94218B7 at francisco-MacBookPro>
Date: Fri,  6 Sep 2019 13:51:02 +0200 (CEST)
From: francisco <francisco at francisco-MacBookPro>

make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Building application "hello-world" for "native" with MCU "native".

"make" -C /home/francisco/workspace/RIOT/boards/native
"make" -C /home/francisco/workspace/RIOT/boards/native/drivers
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/native
"make" -C /home/francisco/workspace/RIOT/cpu/native/periph
"make" -C /home/francisco/workspace/RIOT/cpu/native/vfs
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
   text	   data	    bss	    dec	    hex	filename
  22942	    568	  47652	  71162	  115fa	/home/francisco/workspace/RIOT/examples/hello-world/bin/native/hello-world.elf
make: Leaving directory '/home/francisco/workspace/RIOT/examples/hello-world'
```
</details>

I think the old test procedure of testing `TERM=at` worked because `tput` is based on `TERM`, and since with the new change we aren't checking tput the old test procedure didn't work.

So properly reproducing the test procedure I saw in #8007 this PR doesn't seem to brake any use case and it does fix the bug described in the PR.

<details><summary> <b>pr: </b>I_SHOULD_NOT_HAVE_COLORS 
</summary>

```
make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
Building application "hello-world" for "samr21-xpro" with MCU "samd21".

"make" -C /home/francisco/workspace/RIOT/boards/samr21-xpro
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/samd21/periph
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/newlib_syscalls_default
"make" -C /home/francisco/workspace/RIOT/sys/pm_layered
"make" -C /home/francisco/workspace/RIOT/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
   8508	    120	   2560	  11188	   2bb4	/home/francisco/workspace/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf
make: Leaving directory '/home/francisco/workspace/RIOT/examples/hello-world'
```
</details>
<details><summary> <b>master: </b> I_SHOULD_NOT_HAVE_COLORS 
</summary>

```
make: Entering directory '/home/francisco/workspace/RIOT/examples/hello-world'
^[[1;32mBuilding application "hello-world" for "samr21-xpro" with MCU "samd21".^[[0m

"make" -C /home/francisco/workspace/RIOT/boards/samr21-xpro
"make" -C /home/francisco/workspace/RIOT/core
"make" -C /home/francisco/workspace/RIOT/cpu/samd21
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common
"make" -C /home/francisco/workspace/RIOT/cpu/cortexm_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common
"make" -C /home/francisco/workspace/RIOT/cpu/sam0_common/periph
"make" -C /home/francisco/workspace/RIOT/cpu/samd21/periph
"make" -C /home/francisco/workspace/RIOT/drivers
"make" -C /home/francisco/workspace/RIOT/drivers/periph_common
"make" -C /home/francisco/workspace/RIOT/sys
"make" -C /home/francisco/workspace/RIOT/sys/auto_init
"make" -C /home/francisco/workspace/RIOT/sys/newlib_syscalls_default
"make" -C /home/francisco/workspace/RIOT/sys/pm_layered
"make" -C /home/francisco/workspace/RIOT/sys/stdio_uart
   text    data     bss     dec     hex filename
   8504     120    2560   11184    2bb0 /home/francisco/workspace/RIOT/examples/hello-world/bin/samr21-xpro/hello-world.elf
make: Leaving directory '/home/francisco/workspace/RIOT/examples/hello-world'
```
</details>





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/12127#pullrequestreview-284837536
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190906/99f4d545/attachment-0001.htm>


More information about the notifications mailing list