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

Add support for Apple Swift language #1323

Merged
merged 1 commit into from May 27, 2019

Conversation

@ankitpati
Copy link
Contributor

commented Nov 26, 2016

Changes requested in review of #1265 incorporated.

Copy link
Member

left a comment

Okay, almost there :)

[build-menu]
FT_00_LB=
FT_00_CM=
FT_00_WD=

This comment has been minimized.

Copy link
@b4n

b4n Nov 26, 2016

Member

you should remove the empty entry and move the next ones in place (e.h. FT_01FT_00 and FT_02FT_01)

@@ -61,6 +61,7 @@ filetypes = \
filedefs/filetypes.ruby \
filedefs/filetypes.rust \
filedefs/filetypes.Scala.conf \
filedefs/filetypes.Swift.conf \

This comment has been minimized.

Copy link
@b4n

b4n Nov 26, 2016

Member

not very important, but we sort them alphabetically and case insensitively, so it should go after filetypes.sql

@b4n b4n self-assigned this Nov 26, 2016
@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

Incorporated changes requested in the review.

@b4n
b4n approved these changes Nov 26, 2016
@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

Thank you for the approval. Should I squash the commits now?

@b4n

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

You can, or you can let us do it when merging: either way is fine.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

I am fine with either. What is the norm here?

@b4n

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

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.

@elextr

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

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++.

@b4n

This comment has been minimized.

Copy link
Member

commented Nov 26, 2016

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.

@ankitpati ankitpati force-pushed the ankitpati:apple-swift-support branch from 4a9370e to ddf2def Nov 26, 2016
@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Nov 26, 2016

Squashed and force-pushed.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2016

@b4n May I seek your attention for this PR?

There is a trivial merge conflict. Should I fix it?

@elextr

This comment has been minimized.

Copy link
Member

commented Dec 28, 2016

@ankitpati though the conflict is trivial, might as well fix it, the less for committers to do the more likely it will happen.

LGBI

@ankitpati ankitpati force-pushed the ankitpati:apple-swift-support branch from ddf2def to 2a86cad Dec 28, 2016
@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2016

Resolved. Please consider merging this in at the earliest.

@b4n

This comment has been minimized.

Copy link
Member

commented Dec 29, 2016

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 guess it's better to have partial support for a language than no support at all. 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.

@elextr

This comment has been minimized.

Copy link
Member

commented Dec 29, 2016

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.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 29, 2016

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.

@elextr

This comment has been minimized.

Copy link
Member

commented Dec 29, 2016

@ankitpati the main missing feature is the symbols parser.

Geany gets these from Universal ctags which is a separate project. Hopefully that project has some documentation on how to add parsers so you should look there first.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 29, 2016

Thank you. Looking into it.

@codebrainz

This comment has been minimized.

Copy link
Member

commented Dec 29, 2016

@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:

keith/swift.vim@5d5f814

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.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 30, 2016

@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.

@elextr

This comment has been minimized.

Copy link
Member

commented Dec 30, 2016

@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 separately.

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 30, 2016

@elextr Agreed. @b4n Any thoughts?

@b4n

This comment has been minimized.

Copy link
Member

commented Dec 30, 2016

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). 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:

@ankitpati

This comment has been minimized.

Copy link
Contributor Author

commented Dec 30, 2016

test stuff when you don't have a clue how to install swift

I could have helped with that.

After that, you may actually use filetype_extensions.conf and filetypes.Swift.conf from this very PR to get Swift support in Geany.

@ntrel

This comment has been minimized.

Copy link
Member

commented May 17, 2019

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.

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

This comment has been minimized.

Copy link
@ntrel

ntrel May 19, 2019

Member

Could also add this for """multiline string""" highlighting:

[lexer_properties]
lexer.cpp.triplequoted.strings=1

This comment has been minimized.

Copy link
@ankitpati

ankitpati May 19, 2019

Author Contributor

Done.

@ankitpati ankitpati force-pushed the ankitpati:apple-swift-support branch from 2a86cad to 4ad1b85 May 19, 2019
@ntrel

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Thanks @ankitpati.

@b4n et al: OK to merge? (got thumbs up from @codebrainz on my previous comment).

@elextr

This comment has been minimized.

Copy link
Member

commented May 22, 2019

LGBI

@ntrel ntrel merged commit 95c0c06 into geany:master May 27, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.