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

asm: Add .equiv define #1557

Merged
merged 3 commits into from
Sep 26, 2017
Merged

asm: Add .equiv define #1557

merged 3 commits into from
Sep 26, 2017

Conversation

markferry
Copy link
Contributor

@markferry markferry commented Sep 21, 2017

Simple change to recognize .equiv assembler directive.
(http://ftp.gnu.org/old-gnu/Manuals/gas/html_chapter/as_7.html#SEC87)

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.941% when pulling 7e693ed on cognomen:asm-equiv into b953728 on universal-ctags:master.

@masatake
Copy link
Member

Thank you.

  1. could you add the reference "ftp://ftp.gnu.org/old-gnu/Manuals/gas/html_chapter/as_7.html#SEC87"
    not only to the comment of this pull request but also the commit log. It is helpful to understand the change with git log and git blame.

  2. could you add a test case to Units/parser-asm.r/ ?
    See http://docs.ctags.io/en/latest/units.html to know how to write a test case.

@masatake masatake requested review from masatake and removed request for masatake September 21, 2017 18:07
@masatake masatake self-assigned this Sep 22, 2017
@masatake
Copy link
Member

Please, forget my lsat comment. I will make a pull request to your branch.

@masatake
Copy link
Member

I guess you know well about gas.
How do you think about .eqv directive?


7.26 `.eqv SYMBOL, EXPRESSION'
==============================

The `.eqv' directive is like `.equiv', but no attempt is made to
evaluate the expression or any part of it immediately.  Instead each
time the resulting symbol is used in an expression, a snapshot of its
current value is taken.

@markferry
Copy link
Contributor Author

Thanks @masatake. I hadn't noticed .eqv. Looks like it's appropriate to incorporate it. I'll also add test cases and better commit messages. Updated patches to follow...

@masatake
Copy link
Member

O.k. I change my mind again. I will wait for your updating.
Feel free to ask me any question about writing a test case.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 85.945% when pulling 0e2e8f4 on cognomen:asm-equiv into b953728 on universal-ctags:master.

@masatake masatake merged commit e1bc2a3 into universal-ctags:master Sep 26, 2017
@masatake
Copy link
Member

Thank you very much. I squashed three commits into one. (I made a typo in the commit log. Sorry.)

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.

None yet

3 participants