Changes requested in review of #1265 incorporated. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/1323
-- Commit Summary --
* Add support for Apple Swift language
-- File Changes --
M data/Makefile.am (1) A data/filedefs/filetypes.Swift.conf (45) M data/filetype_extensions.conf (3)
-- Patch Links --
https://github.com/geany/geany/pull/1323.patch https://github.com/geany/geany/pull/1323.diff
b4n requested changes on this pull request.
Okay, almost there :)
+# the following characters are these which a "word" can contains, see documentation +#wordchars=_abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 + +# single comments, like # in this file +comment_single=// +# multiline comments +comment_open=/* +comment_close=*/ + +comment_use_indent=true + +[build-menu] +FT_00_LB= +FT_00_CM= +FT_00_WD=
you should remove the empty entry and move the next ones in place (e.h. `FT_01` → `FT_00` and `FT_02` → `FT_01`)
@@ -61,6 +61,7 @@ filetypes = \
filedefs/filetypes.ruby \ filedefs/filetypes.rust \ filedefs/filetypes.Scala.conf \ + filedefs/filetypes.Swift.conf \
not very important, but we sort them alphabetically and case insensitively, so it should go after `filetypes.sql`
@ankitpati pushed 2 commits.
2a76f95 place Swift after sql (sort alphabetically) 4a9370e removed empty entries from [build-menu]
Incorporated changes requested in the review.
b4n approved this pull request.
Thank you for the approval. Should I squash the commits now?
You can, or you can let us do it when merging: either way is fine.
I am fine with either. What is the norm here?
We generally like to ask PR authors to do it for two reasons, one good, and one bad: * The good reason is that, if there are many commits and not everything should be squashed together but only some fixes to specific commits, the author is more likely to easily know what fix should got to what commit. * The bad reason is that GitHub doesn't provide a way to mark PRs as merged if squashing manually (e.g. not using their squash button) and not pushed to the PR branch. Best we can do is close them.
However, we are aware that not everybody is comfortable enough with Git to squash manually without fear or trial and error, so ultimately we are happy to do it for anyone not comfortable doing it him/herself.
Also, last thing: we always prefer squashing to happen at the very end of a PR review process, when everything is sorted out. Squashing commits during review iterations makes it harder to see changes for reviewers, so apart for really simple commits it's better to wait for approval. You've got mine for this one so it's fine though :)
So, **the summary is**: if you have no problem doing it, please, do. Otherwise, don't worry for a second and we'll do it ourselves.
Did you try the C++ lexer as I suggested on your previous PR? I don't know swift, but as I noted the support in Scite (which is from the author of Scintilla) seems to use c++.
Did you try the C++ lexer as I suggested on your previous PR? I don't know swift, but as I noted the support in Scite (which is from the author of Scintilla) seems to use c++.
There's only really one lexer for C and C++ in Scintilla: `SCLEX_CPP`.
Squashed and force-pushed.
@b4n May I seek your attention for this PR?
There is a trivial merge conflict. Should I fix it?
@ankitpati though the conflict is trivial, might as well fix it, the less for committers to do the more likely it will happen.
LGBI
Resolved. Please consider merging this in at the earliest.
LGBI.
@codebrainz raised concerns on IRC about having "imperfect" filetype (no symbols and related features like autocompletion and calltips) in core and that people would complain it's broken. I don't really know how used Swift is, and I don't really think it's bets not to have support for a language than having partial one. Opinions? @elextr
BTW, I started a CTags parser for this, but I won't likely have it ready too quickly. But @codebrainz suggested maybe someone could quickly write a regex-based parser based off some ctags command line. Should be fairly easy I think.
Well, this PR will give at least "programmers editor" level of support, even if it has no symbols, so it is a useful thing.
I don't know swift, but IIUC its a derivative from obj-c++ so I would wonder how well a regex based parser would work. Such a parser would not work on C/C++ of course.
I would like to help with adding at least some of what is missing. I would appreciate any pointers on where and how I can get started.
@ankitpati the main missing feature is the symbols parser.
Geany gets these from [Universal ctags](https://github.com/universal-ctags/ctags) which is a separate project. Hopefully that project has some documentation on how to add parsers so you should look there first.
Thank you. Looking into it.
@ankitpati, these are some useful docs for adding parsers from Exuberant CTags project (which Universal CTags is a fork of), I presume they mostly still apply:
http://ctags.sourceforge.net/parser.html http://ctags.sourceforge.net/EXTENDING.html
There's this VIM config file which shows the command-line based custom Swift regex parser some people use (no idea how well it works), which could be ported to a compiled regexp parser as per above docs:
https://github.com/keith/swift.vim/commit/5d5f81483d1ecc611c0dcd2846aab872f0...
According to @b4n the main limitation with a regexp-based parser is that it can't handle stuff like scope information, while a character-based parser can support this since it can maintain own state during the parse, if I understood correctly.
@codebrainz It is possible to maintain state with regex using capturing groups and look-arounds, although the feasibility of implementing a complex parser is debatable.
@ankitpati note that @b4n mentioned above starting a parser, maybe you should lean on him to put his "work in progress" on github so you can assist with that. Better than two people working on it.
@elextr Agreed. @b4n Any thoughts?
Yeah sure. I pushed the current state I have to https://github.com/b4n/fishman-ctags/tree/wip/parsers/swift (it's against [Universal-CTags](https://github.com/universal-ctags/ctags)). I'm not sure what's the exact state, but it surely is full of holes. Also beware, it uses features not yet imported in Geany, so integrating it will require merging #1263 first.
Useful stuff I used: * More or less strict language reference: https://developer.apple.com/library/content/documentation/Swift/Conceptual/S... * test stuff when you don't have a clue how to install swift: https://swiftlang.ng.bluemix.net/#/repl
test stuff when you don't have a clue how to install swift
I could have [helped](https://swift.org/download/#using-downloads "Swift.org") with that.
After that, you may actually use [`filetype_extensions.conf`](https://raw.githubusercontent.com/ankitpati/geany/2a86cad6f3219e02a967a357da... "PR 1323") and [`filetypes.Swift.conf`](https://raw.githubusercontent.com/ankitpati/geany/2a86cad6f3219e02a967a357da... "PR 1323") from this very PR to get Swift support in Geany.
I guess it's better to have partial support for a language than no support at all. Opinions?
Absolutely, highlighting without tag parsing is fine. And once highlighting is in, people will start to use Swift with Geany, so someone else may step up to add tag parsing.
ntrel commented on this pull request.
@@ -0,0 +1,42 @@
+# For complete documentation of this file, please see Geany's main documentation +[styling=C] + +[keywords] +primary=associativity break case catch class continue convenience default deinit didSet do else enum extension fallthrough false final for func get guard if import in infix init inout internal lazy let mutating nil operator override postfix precedence prefix private public repeat required return self set static struct subscript super switch throws true try var weak where while willSet +secondary=Array Bool Dictionary ErrorType Int Float Double Set String Tuple UnicodeScalar abs max min print +# documentation keywords for javadoc +doccomment=author deprecated exception param return see serial serialData serialField since throws todo version +
Could also add this for `"""multiline string"""` highlighting: ```ini [lexer_properties] lexer.cpp.triplequoted.strings=1 ```
ankitpati commented on this pull request.
@@ -0,0 +1,42 @@
+# For complete documentation of this file, please see Geany's main documentation +[styling=C] + +[keywords] +primary=associativity break case catch class continue convenience default deinit didSet do else enum extension fallthrough false final for func get guard if import in infix init inout internal lazy let mutating nil operator override postfix precedence prefix private public repeat required return self set static struct subscript super switch throws true try var weak where while willSet +secondary=Array Bool Dictionary ErrorType Int Float Double Set String Tuple UnicodeScalar abs max min print +# documentation keywords for javadoc +doccomment=author deprecated exception param return see serial serialData serialField since throws todo version +
Done.
Thanks @ankitpati.
@b4n et al: OK to merge? (got thumbs up from @codebrainz on my previous comment).
LGBI
Merged #1323 into master.
github-comments@lists.geany.org