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

autoit: Drop $ from variable names #3697

Merged
merged 2 commits into from
Apr 27, 2023
Merged

Conversation

techee
Copy link
Contributor

@techee techee commented Apr 16, 2023

At least PHP, which also declares variables using $, such as

$x = 5;

outputs pure variable name without the $ prefix and this patch unifies this behavior.

At least PHP, which also declares variables using $, such as

$x = 5;

outputs pure variable name without the $ prefix and this patch unifies
this behavior.
@techee
Copy link
Contributor Author

techee commented Apr 16, 2023

Autocompletion doesn't work for variables in Geany because of this, see geany/geany#3460. We could add a workaround if having the $ is really the desired behavior though I think as a rule, ctags doesn't add these prefix characters to generated tags.

@codecov
Copy link

codecov bot commented Apr 16, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (5ba7dfc) 82.82% compared to head (85fa80a) 82.82%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3697   +/-   ##
=======================================
  Coverage   82.82%   82.82%           
=======================================
  Files         226      226           
  Lines       54750    54752    +2     
=======================================
+ Hits        45349    45351    +2     
  Misses       9401     9401           
Impacted Files Coverage Δ
parsers/autoit.c 99.28% <100.00%> (+0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@masatake
Copy link
Member

masatake commented Apr 18, 2023

Thank you. This makes sense. However, the impact is not small; there can be a client tool that expects names start from $.

Since version 6.0.0, I introduced per-parser versioning. This is a good time to utilize it.

struct sParserDefinition {
	/* defined by parser */
	char* name;                    /* name of language */

	/* The concept of CURRENT and AGE is taken from libtool.
	 * However, we deleted REVISION in libtool when importing
	 * the concept of versioning from libtool.
	 *
	 * If kinds, roles, fields, and/or extras have been added,
	 * removed or changed since last release, increment CURRENT.
	 * If they have been added since last release, increment AGE.
	 * If they have been removed since last release, set AGE to 0
	 *
	 * From the command line of ctags, you can see the version
	 * information with --version=<LANG>.
	 *
	 * In the tags file, !_TAGS_PARSER_VERSION!<LANG> shows the
	 * information for <LANG>.
	 */
	unsigned int    versionCurrent;
	unsigned int    versionAge;

Though your change doesn't match the criteria I wrote as the comment, obviously this is the
time to increment the current and set the age to 0.

Could you add the following change to your commit?

diff --git a/parsers/autoit.c b/parsers/autoit.c
index 8cc9b6386..24b2ee116 100644
--- a/parsers/autoit.c
+++ b/parsers/autoit.c
@@ -313,5 +313,7 @@ parserDefinition *AutoItParser (void)
 	def->extensions = extensions;
 	def->parser     = findAutoItTags;
 	def->useCork    = CORK_QUEUE;
+	def->versionCurrent = 1;
+	def->versionAge    = 0;
 	return def;
 }

I will add a man page for the parser to explain this change, like https://docs.ctags.io/en/latest/man/ctags-lang-kconfig.7.html, later.

@masatake
Copy link
Member

I'll take over this.

@techee
Copy link
Contributor Author

techee commented Apr 24, 2023

I'll take over this.

OK, great.

The change in the last commit for the parser is not small.
So I increment the current, and set 0 to age.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake masatake merged commit 7ebd379 into universal-ctags:master Apr 27, 2023
@masatake
Copy link
Member

Thank you.

techee added a commit to techee/geany that referenced this pull request Apr 28, 2023
techee added a commit to techee/geany that referenced this pull request May 7, 2023
techee added a commit to techee/geany that referenced this pull request Oct 4, 2023
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