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.

Sign up for GitHub

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

Jump to bottom

Few improvements #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Few improvements #5

wants to merge 3 commits into from

Conversation

im-0
Copy link
Contributor

@im-0 im-0 commented Oct 30, 2020

No description provided.

@im-0
Make generate_compdb.py executable
e4e23ae
@im-0
Put compile_commands.json into .vscode directory by default
8255b8c 
This makes more sense because:

* No need in adding it into kernel's .gitignore
* It will not be mistaken with kernel's source files
@im-0
Add note about intelliSenseMode
ed37532
amezin
amezin requested changes Oct 30, 2020
View reviewed changes
generate_compdb.py Show resolved Hide resolved
generate_compdb.py
cmd_parser.add_argument(
'-o', '--output',
type=str, default=default_output,
help="Path to resulting JSON file, default: %s" % (default_output, ))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use ArgumentDefaultsHelpFormatter instead of formatting help strings by hand. It's available even in Python 2. You won't need default_* variables with it.

@amezin
Copy link
Owner

amezin commented Jan 9, 2021

Merged some of your changes by cherry-picking

I'm still not sure about moving compile_commands.json. At least about doing so by default

  1. All c/c++ language servers that I'm aware of are searching for compile_commands.json in the root of the source tree by default
  2. https://github.com/torvalds/linux/blob/master/.gitignore#L154

I'm ok with adding --output option. Could you add the option as a separate commit, please? (without adding defaults to the help string, without moving the output file by default)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amezin amezin amezin requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@im-0 @amezin