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

Per parser param option #1166

Merged
merged 15 commits into from
Nov 13, 2016

Conversation

masatake
Copy link
Member

(test cases for --list-params and documents for --param-= come later.)

@pragmaware, please, look at 3c4f51f, 523db96 and 374bab3.

Newly introduced parameterHandlerTable field of parserDefinition handles per parser parameters.
option.c calls applyParameter to set the value for a paraemter given via command line.

@pragmaware
Copy link
Contributor

pragmaware commented Oct 14, 2016

Yep, this could work.

Though I must say that --param-<LANG>=name:arg looks quite complicated.

Think of --param-CppPreProcessor=ignore:MACRO(a,b)=replacement ... this is very hard to write down properly without several trial-error-man-page-lookup cycles.

Maybe --param-<LANG>-name=arg instead should be used (and it would be nice to accept the --param-<LANG>-name arg form too).

And in the end I think --param-CppPreProcessor-ignore=MACRO(a,b)=replacement should be really aliased to -D MACRO(a,b)=replacement, to mimick the gcc usage. But that can probably be done just like you did with -I now.

Ah... and in the preprocessor 'ignore' should eventually become 'define' instead. We'll not be ignoring these tokens anymore, we'll be eventually defining them to an empty string. But that can be done later.

@masatake
Copy link
Member Author

Thank you for reading.

Though I must say that --param-=name:arg looks quite complicated.

Think of --param-CppPreProcessor=ignore:MACRO(a,b)=replacement ... this is very hard to write down properly without several trial-error-man-page-lookup cycles.

Maybe --param--name=arg instead should be used (and it would be nice to accept the --param--name arg form too).

I'm thinking about using completion in input. I have never tried but --param-<LANG>=name:arg may helpful to limit the number of candidates printing at once. - char can be part of . (I would like to reserve - and _ as characters being part of ). So the name part of --param-<LANG>-name cannot be extracted easily.

How about --param-CppPreProcessor/ignore=MACRO(a,b)=replacement ?
None at POSIX environment wants to use / in .

Experimentally I will write a bash completion rule for ctags; and evaluate what I wrote here is meaningful or not.

How about backend mechanism, applyParameter and parameterHandlerTable ? Do they help you implement ideas you have about Cxx parsers? I hope so.

And in the end I think --param-CppPreProcessor-ignore=MACRO(a,b)=replacement should be really aliased to -D MACRO(a,b)=replacement, to mimick the gcc usage. But that can probably be done just like you did with -I now.

Ah... and in the preprocessor 'ignore' should eventually become 'define' instead. We'll not be ignoring these tokens anymore, we'll be eventually defining them to an empty string. But that can be done later.

Yes. My requests are

  • Keep the compatibility about what -I accepts,
  • Introduce -D if you need to keep the compatiblity (BTW, I would like to use -D option for the same parpose in m4 parser :-)
  • use applyParameter and parameterHandlerTable as backend for them

The name of backend parameter, "ignore" is just temporary name. You can rearrange suitable for implementing -I and -D; renaming, and adding new one are o.k., of course.

However, when releasing the name of parameter is also the first class command line interface. Ideally it should be stable. If you would like to introduce an experimental parameter, use _ as prefix.
I will add code which hides parameters started with _ from the output of --list-params.

The topic you are not satisfied with is the separater (notation) of --param-<LANG>....
Am I correct?

@masatake
Copy link
Member Author

How about using . or : as a separator?

--param-CppPreProcessor:ignore=MACRO(a,b)=replacement
--param-CppPreProcessor.ignore=MACRO(a,b)=replacement

These are completion friendly.

(In the discussion on github I called this as --opt-<LANG>. However,
 "opt" and "option" are used so widely in ctags. To avoid confusion,
 I use "param" instead.)

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
This commit introduces --param-CPreProcessor=ignore:TOKEN.
Now -I option is a thin wrapper for --param-CPreProcessor=ignore:TOKEN.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
It is already set in initTagEntryFull.
No need to do twice.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
--if0 optoin is still available but it is thin wrapper for --param-CPreProcessor=if0:true.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
…: option

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Signed-off-by: Masatake YAMATO <yamato@redhat.com>
Revised format:

	--param-<LANG>:<PARAM>=VALUE

* Use `:' for separator between <LANG> and <PARAM>.
* Use `=' for separator between <PARAM> and <VALUE>.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake
Copy link
Member Author

I chose `:'.

@masatake masatake changed the title [RFC] Per parser param option Per parser param option Nov 10, 2016
@masatake
Copy link
Member Author

@pragmaware,

--param-CppPreProcessor:ignore=MACRO(a,b)=replacement

is acceptable?

@pragmaware
Copy link
Contributor

Looks fine for me. Should I merge?

@masatake masatake merged commit 0b1b759 into universal-ctags:master Nov 13, 2016
@masatake
Copy link
Member Author

@pragmaware, thank you.
If you are o,k., I will reconstruct #1161 based on this param facility and open a pull request.

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

Successfully merging this pull request may close these issues.

2 participants