ZachMassia / PlatformIO-Mode

PlatformIO Integration for Emacs
GNU General Public License v3.0
68 stars 20 forks source link

Regenterate clang completion file for irony-mode etc. #8

Closed thomasf closed 7 years ago

thomasf commented 7 years ago

If I understand it correctly one has to run platformio init --ide emacs to regenerate the .clang_complete file. Do you know if there some way to tell platformio to only generate that specific template?

I'm all new to platformio, I have just used it with arduino a couple of hours but it seems to me that something should maybe be added to this emacs package for regenerating those clang completions to get irony-mode working properly.

thomasf commented 7 years ago

Btw, this was the least intrusive way I (quickly) found to use the platformio codebase to regenerate that file..

from platformio.ide.projectgenerator import ProjectGenerator

class UpdateClangCompletionsProjectGenerator(ProjectGenerator):

    def get_tpls(self):
        tpls = super(UpdateClangCompletionsProjectGenerator, self).get_tpls()
        new_tpls = []
        for tpl_relpath, tpl_path in tpls:
            if tpl_path.endswith(".clang_complete.tpl") or tpl_relpath.endswith(".clang_complete.tpl"):
                new_tpls.append((tpl_relpath, tpl_path))
        return new_tpls

p = UpdateClangCompletionsProjectGenerator(".", "emacs", None)

p.generate()
ZachMassia commented 7 years ago

Yes, we currently need to re-run init to regenerate the .clang_complete file.

Once you re-init, are you still having issues with irony-mode, or is it only after installing a new library?

When I originally wrote this, I based it off of existing IDE integrations at the time. They also had the problem of having to re-init. Perhaps there is a better way now, or we could talk to Ivan regarding some way to update a specific template file.

I quickly looked through a few other IDE integrations in the current codebase, and I'm not seeing anyone tying in to the internal Python code. Given that currently the emacs template is only two files, one being a static .gitignore file, I'm not sure there is much benefit to adding the complexity of having to tie into Python from the mode's elisp code.

I'm certainly open to streamlining the whole library installation / .clang_complete syncing process though.

ivankravets commented 7 years ago

or we could talk to Ivan

Hey all :)

How I can help here?


P.S: Why you don't use platformio init --ide emacs instead https://github.com/ZachMassia/PlatformIO-Mode/issues/8#issuecomment-264888033?

PIO will update only .clang_complete file and .gitignore will be merged. See https://github.com/platformio/platformio-core/blob/develop/platformio/ide/projectgenerator.py#L125

Also, we have related issues for Project Generator https://github.com/platformio/platformio-core/issues?utf8=✓&q=is%3Aissue%20is%3Aopen%20label%3AIDE%20

ZachMassia commented 7 years ago

Hey,

That was fast, I didn't even @ mention you :P

I believe that your issue #364 is directly related to this.

A simple solution (at least from an outside perspective, I'm not familiar with the code base) would be to support some kind of hook system. This would allow specifying a command to be ran after installing a new library, or even after compiling.

Some method of registering *.tpl files as needing to be re-generated after changes could also work.

For now I believe the simplest is to just re-run platformio init --ide emacs, correct?

ivankravets commented 7 years ago

That was fast, I didn't even @ mention you :P

Twitter/MELPA notified me :)

A simple solution (at least from an outside perspective, I'm not familiar with the code base) would be to support some kind of hook system.

We use "system directory watcher" in PlatformIO IDE. Do you have some functionality in emacs? This?

See what we monitor in PIO IDE: https://github.com/platformio/platformio-atom-ide/blob/develop/lib/init/command.js#L314

For now I believe the simplest is to just re-run platformio init --ide emacs, correct?

That is correct solution for current version.

ivankravets commented 7 years ago

Also, we monitor "platformio.ini" from project folder. Sometime user changes extra directories, library dependencies and etc.

thomasf commented 7 years ago

@ivankravets I wanted to to only regenerate the file I'm specifically intereseted in but it's probably not all that important. I liked the non verbose output of my small hack better than what the --init ide... outputs but thats maybe avoidable by not displaying stdout at all.

Emacs support fs events so that might be something to look into.

I'm very new to platformio (great stuff, thanks!) so I don't know many of it's details. I think that I actually want to commit my external libraries folders of my projects so that I can build projects without relying on an external package repository which in time can disappear. This means that I do not want my .gitignore to be updated with the .piolibs folder but that is maybe possible to achieve somehow?

ZachMassia commented 7 years ago

@ivankravets, The fs watcher is a possibility. I could add an option to watch platformio.ini for changes, and re-init automatically.

@thomasf I suppose a workaround for now would be putting/copying your libraries into the lib folder in the project root, which is not added to .gitignore. You could also run that Python snippet you wrote, but I don't think for now that I will bring that into platformio-mode.

thomasf commented 7 years ago

@ZachMassia It just took me some time to figure out why irony-mode wouldn't work. It was only after reading the PIO pytnon source code I found the clang completion template and understood that platformio init... was the way to generate it.. I also had some problems with getting irony-mode running.. At the moment I have added irony-cdb-autosetup-compile-options to my platformio-mode-hook for that part to work as well but thats maybe some user error on my part.

I guess I would probably want a -s(ilent) argument for platformio -init... and a section for excluding ide template files in platformio.ini to avoid generating the .gitignore at all. Seems like a couple of trivial things to do...

ivankravets commented 7 years ago

Could someone explain me which problems do you have? platformio init --ide emacs doesn't touch your libraries or modify source code. The only 1 file that will be modified is .clang_complete. I've just tried on demo project multiple times with custom .gitignore. PIO doesn't overwrite .gitignore.

Please don't do manual and unnecessary steps. PIO Project Generator runs PIO Build System in special mode to fetch real build environement including CPPATH, build flags and etc. You should use high level API (platformio.ini) and specify custom library storages here. See http://docs.platformio.org/en/stable/projectconf.html#projectconf-lib-extra-dirs

P.S: I've just added [libsource_dirs](https://github.com/platformio/platformio-core/commit/9483c0c51f08d333c73f526e4f72f844ce1fc8e4] to exported data for IDE. It could be useful for Dir Watchers. Please note, that PIO 3.2 has not been released yet/ I hope we will release it tomorrow. Summary, you will need to monitor platformio.ini and all dirs from libsource_dirs. In this case, any IDE/Emacs will rebuild C/C++ Index automatically.

thomasf commented 7 years ago

My problem is.

  1. I have added `.piolibdeps' to my git repoistory

2 I have removed piolobdeps from my .gitignore file

  1. I run platformio init --ide emacs

  2. piolobdeps is back into the .gitignore file

I just dont want to annoy other users like I'm annoyed by this by just updating the relevant file for ide integration (the clang completions one) and keep unrelated files as they are.

ivankravets commented 7 years ago

.piolibdeps is PIO service folder and is hidden from user scope. Please don't touch and don't use it. You should use lib_deps option or place PRIVATE library to lib directory located in the root of project.

thomasf commented 7 years ago

Thanks for the reply, the inclusion of src dirs listing is great!

a smallish note on .gitignore merger

The .gitignore merger reorders the .gitignore file so it's not without side effects:

before init:

# my assets
/ASSETS

# pio
.pioenvs
.clang_complete
.piolibdeps

after init:

.pioenvs
.clang_complete
# my assets
/ASSETS
# pio
.piolibdeps

It should not be shuffeled and it most imporantly should not be rewritten unless there are actual content changes.

Can I submit a pull request changing .gitignore merger and adding a feature that if there is a commented line like # .pioenvs it wont be added back?

on checking in vendored dependecien in general

Sorry to say this but it's a bit of an harsh thing to saying that I should not "touch" the .piolibdeps, we don't always have exactly the same use case.

The PIO libs manager is a really great tool and one of the thing which makes PIO fun for me and manually moving between libs and .piolibdeps would become quite annoying.. I can take responsibility myself if keeping .piolibdeps in the repository tree cases me problems.

The main reason for me to start using PIO is to be able to share certain projects with other people and be able to use them easily in Darwin or Windows environments if that becomes something I need to do.

My main concert comes down to that I don't want to rely on a centralized service by the grace of one software company's existence whether it's platformio, arduino or github. I want to be able to if everything else fails to just revert back to plain old Makefiles. I have been bitten by this kind of single point of failure too many times to accepting it happening again.

For systems which utilizes well defined package mangers (apt/pypi/npm/...) there are some quite good specialized caching proxy servers now which assists the archiving process.

thomasf commented 7 years ago

@ivankravets would you accept a pio-core pull request which passes this sketch for a test and also don't rewrite any files in ProjectGenerator_merge_contents if the contents has not changed changed?


#  @staticmethod to be added into ProjectGenerator
def _merge_gitignore(base, contents):
    """_merge_gitignore returns a contents string if the merge results in any
changes. If a user has commented out a line it will not be readded by the merge.
If there are no changes the returned string is equal to the base argument."""
    result = []
    # ... code...
    return result

contents = '.pioenvs\n.clang_complete\n'

input1 = """# pio
.pioenvs
.clang_complete
.piolibdeps

# my assets
/ASSETS
"""
output1 = input1

input2 = """# pio
.pioenvs
.clang_complete
# .piolibdeps

# my assets
/ASSETS
"""
output2 = input2

input3 = """# pio
.pioenvs
# .piolibdeps

# my assets
/ASSETS
"""
output3 = """# pio
.pioenvs
# .piolibdeps

# my assets
/ASSETS
.clang_complete
"""

test_data = [
    (input1, output1),
    (input2, output2),
    (input3, output3),
]
thomasf commented 7 years ago

I might get a little bit of time to implement the suggested behaviour above later this evening or later this week.

ivankravets commented 7 years ago

Please read docs. You need http://docs.platformio.org/en/stable/projectconf.html#lib-extra-dirs

ZachMassia commented 7 years ago

I'm going to go ahead and close this. I believe this is more an issue for PIO directly.

thomasf commented 7 years ago

@ivankravets I did see the extra dirs stuff, I'll create an issue in pio-core about the broken .gitignore merger later instead.

thomasf commented 7 years ago

@ZachMassia From what I understand platformio-mode will still need to at least call platformio --init -ide emacs in some situations since it is the ide/editors responsibility to invoke it's specific init ide profile..

I don't think closing this issue is correct?

ivankravets commented 7 years ago

@thomasf please report an issue to PIO repository because we are going to make 3.2 release tomorrow. Thanks.

ZachMassia commented 7 years ago

@thomasf I can add a command similar to the one the Eclipse template has. I'm not sure how much value it will add however, as package installation is currently handled outside of Emacs anyways.

@ivankravets are there any plans to close #364? I'd hate to duplicate work on this when it could be brought into PIO proper and benefit all IDE templates.

thomasf commented 7 years ago

@ZachMassia I think that would be great adding an interactive command for "init" , maybe even naming it something like platformio-init-update-workspace,, since it won't be used for new project initialization it should be clear what it does from it's name while still keeping init- after the package prefix to avoid inventing a whole new language for terms already used in platformio.

If it had been there this weekend it would probably have saved me the trip of starting to read the platformio source to figure out where the clang completion file is created.

ivankravets commented 7 years ago

@ZachMassia

are there any plans to close #364?

I'm going back to PlatformIO IDE. It has not been updated since the first February's release. Need to refactor it and integrate PIO Core Library Manager, PIO Remote and etc.

I recommend adding of new target/command like this https://github.com/platformio/platformio-core/blob/develop/platformio/ide/tpls/eclipse/.cproject.tpl#L177

ZachMassia commented 7 years ago

@thomasf I will try and get the command added either tonight or tomorrow night.

I will also update the docs to make it a little clearer that after installing more libraries, the command must be ran.

It is mentioned in the PIO docs at the bottom in yellow, but I suppose it should also be mentioned in the README here.

ivankravets commented 7 years ago

Thanks! That is good solution!

rileyrg commented 5 years ago

Am I right in thinking this is still the situation? We need to manually regenerate clang_complete?