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

./configure output readability #595

Closed
mscdex opened this issue Jan 24, 2015 · 9 comments
Closed

./configure output readability #595

mscdex opened this issue Jan 24, 2015 · 9 comments

Comments

@mscdex
Copy link
Contributor

mscdex commented Jan 24, 2015

Since 1b1cd1c the ./configure output has gotten worse (I actually didn't mind the behavior before that commit). IMHO it's a step backwards.

This is what I see now:

{'variables': {'node_shared_http_parser': 'false',
'want_separate_host_toolset': 0, 'icu_small': 'false', 'node_use_mdb':
'false', 'target_arch': 'ia32', 'host_arch': 'ia32', 'v8_optimized_debug': 0,
'node_install_npm': 'true', 'openssl_no_asm': 0, 'uv_parent_path':
'/deps/uv/', 'v8_no_strict_aliasing': 1, 'node_shared_zlib': 'false',
'node_use_dtrace': 'false', 'python': '/usr/bin/python', 'v8_enable_gdbjit':
0, 'node_use_etw': 'false', 'v8_enable_i18n_support': 0, 'node_use_perfctr':
'false', 'v8_random_seed': 0, 'node_prefix': '', 'node_shared_openssl':
'false', 'uv_use_dtrace': 'false', 'node_shared_v8': 'false',
'node_use_openssl': 'true', 'uv_library': 'static_library', 'v8_use_snapshot':
'true', 'node_shared_libuv': 'false', 'node_tag': ''}, 'target_defaults':
{'libraries': [], 'default_configuration': 'Release', 'cflags': [],
'include_dirs': [], 'defines': []}}

Surely there is a better way? The keys aren't even in alphabetical order.

@silverwind
Copy link
Contributor

Could definitly be improved by proper nesting and sorting.

@rvagg
Copy link
Member

rvagg commented Jan 25, 2015

context: this was done because there was a new warning introduced for armv6 builds and it appears at the top of this block of output, hence the desire to shorten it because it generally overflows a standard terminal height.

I do agree that it is a bit of an eyesore now though, @trevnorris also commented about this

@mscdex
Copy link
Contributor Author

mscdex commented Jan 25, 2015

The warning can't be printed after the config option list?

@rvagg
Copy link
Member

rvagg commented Jan 25, 2015

not easily but you're welcome to refactor

@silverwind
Copy link
Contributor

Yeah, the reason for that change was that me and another user missed the warning as it scrolled off because of that long options list.

One possible solution could be to accumulate warnings and errors in a variable and print them at the end. Only issue I have with that would be that they are possibly taken out of context on which step they were generated on. Or alternatively, move the flags to the top (if possible).

@silverwind
Copy link
Contributor

After giving this more thought, I'd propose this:

  • restore options print to its former pprint form
  • add a final print before exiting if warnings happened before, e.g. "Careful, warnings happened above"

This will require a custom warn() function to be used throughout the file. I'll look into doing another PR for that.

@bnoordhuis
Copy link
Member

@silverwind You may want to base it off #585.

@silverwind
Copy link
Contributor

@bnoordhuis Thanks, that PR contains partly what I would've done. Will wait a day or two for that PR to resolve, and the submit my own.

@bnoordhuis
Copy link
Member

Fixed by 6ad236c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants