Skip to content

main: Introduce "--" as option terminator #1884

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

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

Conversation

masatake
Copy link
Member

Close #1883.

To pass a file name started from "-" to the ctags command easier, this
change introduces "--" as option terminator in the command line
processing.

$ cat ./-i
#!/bin/sh
function bar
{
:
}
$ ../../ctags -G -o - -i
ctags: No files specified. Try "ctags --help".
$ ../../ctags -G -o - ./-i
bar	./-i	/^function bar$/;"	f
$ ../../ctags -G -o - -- -i
bar	-i	/^function bar$/;"	f

Signed-off-by: Masatake YAMATO [email protected]

@coveralls
Copy link

coveralls commented Sep 22, 2018

Coverage Status

Coverage decreased (-0.004%) to 84.864% when pulling 7dd5e31 on masatake:introduce-option-terminator into b1621cf on universal-ctags:master.

@masatake masatake force-pushed the introduce-option-terminator branch from 05f92b4 to d17db99 Compare September 22, 2018 16:54
@blueyed blueyed mentioned this pull request Sep 23, 2018
Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thank you!

@masatake
Copy link
Member Author

.... Removing Tmain/nameless-long-option.d test case is not a good idea.
I will revert it partially.

Close universal-ctags#1883.

To pass a file name started from "-" to the ctags command easier, this
change introduces "--" as option terminator in the command line
processing.

    $ cat ./-i
    #!/bin/sh
    function bar
    {
	:
    }
    $ ../../ctags -G -o - -i
    ctags: No files specified. Try "ctags --help".
    $ ../../ctags -G -o - ./-i
    bar	./-i	/^function bar$/;"	f
    $ ../../ctags -G -o - -- -i
    bar	-i	/^function bar$/;"	f

Signed-off-by: Masatake YAMATO <[email protected]>
@masatake masatake force-pushed the introduce-option-terminator branch from d17db99 to 3097f30 Compare September 23, 2018 19:55
@masatake
Copy link
Member Author

"COMMAND LINE INTERFACE" in man/ctags.rst.1.in is the place to update.

Read a list of file names and options from ``file`` (``-`` means
standard input).
File names read using this option are processed after file
names appearing on the command line.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true for options also?
Would rephrase/simplify this then.

standard input).
File names read using this option are processed after file
names appearing on the command line.
Use ``--`` to handle following lines to be handled as file names only.
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a fixup (two times "handle").

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

@blueyed blueyed force-pushed the introduce-option-terminator branch from 9cc40e7 to 7dd5e31 Compare September 24, 2018 00:58
@masatake
Copy link
Member Author

How do you append a change to this pull request?
Very interesting technique.

"--" is not only for "-L -" option. "--" can be used anywhere on a command line.

I'm calling "--" "option terminator". Do you have any better name for it?

if (strncmp (item, "--", (size_t) 2) == 0)

if ((!NonOptionEncountered)
&& (strncmp (item, "--", (size_t) 2) == 0))
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't using the already existing NonOptionEncountered (which IIUC gets set to true when matching a non-option) break passing options after a file name altogether? This would be unfortunate because for now it works fine, and there's even special handling for some options that should be passed before filenames.

Copy link
Member Author

Choose a reason for hiding this comment

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

@b4n, you are correct.
Furthermore, it seems that I broke the compatibility of the command line interface in the past.

Exuberant-ctags:

[jet@localhost]~/var/ctags% ctags  -o - foo.c --list-kinds=Sh          
f  functions

Univeresal-ctags:

[jet@localhost]~/var/ctags% ./ctags  -o - foo.c --list-kinds=Sh
ctags: Warning: cannot open input file "--list-kinds=Sh" : No such file or directory
a	foo.c	/^void a () {}$/;"	f	typeref:typename:void
b	foo.c	/^void b () {}$/;"	f	typeref:typename:void

U-ctags ignores the option (--list-kinds) after input file name.

Another example:

[jet@localhost]~/var/ctags% ctags  -o - foo.c --sort=no      
b	foo.c	/^void b () {}$/;"	f
a	foo.c	/^void a () {}$/;"	f
[jet@localhost]~/var/ctags% ./ctags  -o - foo.c --sort=no      
ctags: Warning: cannot open input file "--sort=no" : No such file or directory
a	foo.c	/^void a () {}$/;"	f	typeref:typename:void
b	foo.c	/^void b () {}$/;"	f	typeref:typename:void

This incompatibility is nothing to do with the newly introduced -- option.
I will do bisect and spot the change that introduced the incompatibility first.
After that, I will revise this pull request.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will work again on this item after reivising #1878.

@masatake
Copy link
Member Author

masatake commented Nov 7, 2018

Bisected.
It seems that I knew this commit breaks compatiblity.

[jet@localhost]~/var/ctags-bisect% git bisect bad                            
f6e092455959f66aa2c43bbaf7591dbe4c77008b is the first bad commit
commit f6e092455959f66aa2c43bbaf7591dbe4c77008b
Author: Masatake YAMATO <[email protected]>
Date:   Wed Jul 1 23:25:56 2015 +0900

    Don't allow putting --list-kinds like options after input file names in command line
    
    Background:
    
          $ ctags -o - main.c --list-kinds
    
    Both the result of parsing main.c and the list of kinds go to stdout.
    A tool consuming output may be confused.
    
    As far as reading option.c ctags doesn't allow such dangerous order of
    option specification.
    
    A boolean, NonOptionEncountered becomes true when the command line parser handles
    an input file name initially.
    
    checkOptionOrder checks the order for short options.
    
    initOnly field of parametricOption and booleanOption structures provides
    the way to declare an order sensitive long options.
    
    These facilities were just ignored.
    I am afraid this behavior was introduced by me when working on optlib.
    However, exuberant ctags shipped as part of Fedora also ignores the order.
    
    This patch fixes the facilities.
    
    This breaks the compatibility between exuberant ctags in semantics of command line
    interpretation.
    
    Signed-off-by: Masatake YAMATO <[email protected]>

:040000 040000 76eae8864a177c8d4c298d8a2048039f4fc2c652 489d56c834550552e5ef7f6c980367fd72e226bf M	main

@masatake
Copy link
Member Author

masatake commented Nov 7, 2018

What I should do first may be updating man/ctags-incompatibilities.7.rst.in.

@blueyed
Copy link
Contributor

blueyed commented Mar 24, 2019

How do you append a change to this pull request?

If the "allow maintainers to change branch" or similar checkbox is checked, others (with write permissions for the repo) can push there, too.

Sorry for the lack of feedback from me - just tried it again, when rg --files would include - in a project, used with rg --files | --links=no -L -. This PR works there, but from quickly looking at the code that might be handled as an option.

And printf '-' | ./ctags --links=no -L - still fails with "ctags: Unknown option: -".

@hirooih
Copy link
Contributor

hirooih commented Feb 12, 2021

@masatake san,

What I should do first may be updating man/ctags-incompatibilities.7.rst.in.

I've sent PR #2859 for this issue (#1889).

If you merge the fix of -- option, we can close #596, #1883, and #1884.

@masatake
Copy link
Member Author

@hirooih, thank you. Your change reduces the complexities behind this pull request.

After merging #2859, I can think about this issue without worrying about the incompatibilities.
I will disallow passing an option via -L - option. In many places, I have assumed the values for options like --kinds-<LANG>=... are never changed during parsing files.

I will rework this item.

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

Successfully merging this pull request may close these issues.

ctags -L: not possible to specifiy files with leading dashes (handled as option)
5 participants