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

[WIP] Fix issue #2009 #2010

Closed
wants to merge 16 commits into from
Closed

[WIP] Fix issue #2009 #2010

wants to merge 16 commits into from

Conversation

eamanu
Copy link
Collaborator

@eamanu eamanu commented Dec 12, 2017

When a new file is saved, this file have the python file "format"

  • Resaltar sintáxis explícitamente.
  • Agregar la feature al menú contextual de ActionBar.
  • Resaltar cuando se guarda un archivo nuevo.

When a new file is saved, this file have the python file "format"
@centaurialpha
Copy link
Member

centaurialpha commented Dec 12, 2017

@eamanu Great! but an additional NEditor is beign created. If you look at the Combo Editor, you will have two files

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 12, 2017

Yes you are right :-(

@centaurialpha
Copy link
Member

We have to find the way to do it generic, the idea is that not only highlight Python code

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 13, 2017

I am not understanding what is the flow. When I pres New File, where the ide add the highlight Python code?

@centaurialpha
Copy link
Member

Perdón voy a escribir en Español, luego lo traduzco.

Por defecto no se crea un Highlighter o resaltado en un archivo nuevo, no sé si es buena idea dejarlo así para luego cuando se guarde o se indique explícitamente qué resaltado usar lo haga, o por defecto resaltar a todo nuevo archivo como Python, ya que no va a ser el único lenguaje que se va a usar en el IDE, la idea es tener soporte para HTML, JS, o cualquiera mediante plugins.

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 13, 2017

Si, anoche estuve leyendo un poco el código y vi mas o menos dónde se podría agregar código para salvar el problemita de este issue, y vi que no se crea automáticamente un Highlighter (me parece perfecto).

Habría que agregar en alguna parte del 'add_editor' o 'save_file_as' (este último me parece ideal) y que lea la extensión del filename y con esto crear el correspondiente Highlighter automáticamente.

@eamanu eamanu changed the title Fix issue #2009 [WIP] Fix issue #2009 Dec 13, 2017
@centaurialpha
Copy link
Member

centaurialpha commented Dec 13, 2017

O mejor aún, agregar esa acción que va a "re-highlightear" esté fuera del save_file_as, ya que como dije, puede ser que este escribiendo un archivo nuevo y sin guardar le indique explícitamente al IDE qué lenguaje estoy usando. Me imaginaba un menú contextual en el Combo para seleccionar el resaltado.

Además tener una referencia al lenguaje que se está usando ya que en base al lenguaje se van a activar cosas como: el Indenter de ese lenguaje, los snippets, el autocompletado, etc..

@centaurialpha
Copy link
Member

Habría que agregar en alguna parte del 'add_editor' o 'save_file_as' (este último me parece ideal) y que lea la extensión del filename y con esto crear el correspondiente Highlighter automáticamente.

Es lo que hace ahí:
https://github.com/ninja-ide/ninja-ide/blob/master/ninja_ide/gui/editor/editor.py#L311

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 13, 2017

Listo, entiendo.

Entonces, los pasos a hacer:

  1. Que cuando crees un file nuevo, en un combo box, o alguna opción se elija el lenguaje para resaltar.
  2. Si no se eligió nada, y se lo guarda al file nuevo, que tome la extensión del archivo que se le dió y de ahí resalte, segùn ese lenguaje.
  3. También se podría hacer que, si se crea un file nuevo, se elije el lenguaje en el cual se quiere trabajar, además de resaltar, cuando lo vaya a guardar lo haga con la extensión propia del lenguaje que eligió.

Que te parece @centaurialpha ?

@centaurialpha
Copy link
Member

Muy bien! Eso es más o menos lo que debería hacer por ahora.

Para tener en cuenta, acá es en donde se construye el Highighter, por ahora sólo tenemos soporte para Python (que podría mejorar, es cuestión de armar las regex)

@pep8speaks
Copy link

pep8speaks commented Dec 16, 2017

Hello @eamanu! Thanks for updating the PR.

Line 501:9: E303 too many blank lines (2)

Comment last updated on December 21, 2017 at 12:28 Hours UTC

Copy link
Member

@centaurialpha centaurialpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Algunos comentarios:

  • Este update no resuelve completamente la issue porque cuando se guarda un archivo nuevo no hace el resaltado.
  • El combobox adicional en el ComboEditor no es necesario. Yo me refería a agregar un submenú en el combo_files (tiene un menu contextual que se abre cuando haces click derecho, actualmente tiene dos acciones) para seleccionar los resaltados de lenguajes soportados por el IDE. La idea es soportar Python, html, js, qml. estos estarían doportados por defecto, y otros mediante plugins.

Si podés sacar ese combobox y mover lo demás a un submenú que esté acá:
screenshot from 2017-12-16 05-01-36
estaría buenisimo, después le vamos dando forma. Perdón que sea jodido, tampoco quiero que dejes de meterle mano al código. Ahí pongo las tareas en el primer comentario. si podes agarrarlas de 10, sino avisá así vamos cerrando la issue.

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 16, 2017

@centaurialpha Ya me pongo a laburar. Mientras más jodido seas, mejor va a salir el Ninja-Ide

@centaurialpha
Copy link
Member

Buen trabajo @eamanu, la segunda tarea está lista 😃

@centaurialpha
Copy link
Member

Obtengo el siguiente error con los últimos cambios:

  File "/home/gabo/Downloads/ninja-ide-fix-2009-issue/ninja_ide/tools/ui_tools.py", line 608, in <lambda>
    item_ui.triggered.connect(lambda _, func=func: func())
  File "/home/gabo/Downloads/ninja-ide-fix-2009-issue/ninja_ide/gui/main_panel/main_container.py", line 223, in save_file
    return self.save_file_as(editor_widget)
  File "/home/gabo/Downloads/ninja-ide-fix-2009-issue/ninja_ide/gui/main_panel/main_container.py", line 270, in save_file_as
    self._setter_language.set_language_from_extension(extension)
  File "/home/gabo/Downloads/ninja-ide-fix-2009-issue/ninja_ide/gui/main_panel/set_language.py", line 54, in set_language_from_extension
    self.dict_extension_language[ext])
KeyError: ''

El dict (dict_extension_language) es basicamente el mismo que está en el módulo del Highlighter, deberíamos agrupar todo eso. Luego te doy una mano con esto, hasta el finde estoy con la facu :p.

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 20, 2017

Ahí resolví el problema del error que me marcaste. Utilicé las funciones del highlighter.py. No lo importé porque me daba error, ya que había importaciones al PyQt4 y el ide usa PyQt5.

También arreglé algunos pep8 issues

@centaurialpha
Copy link
Member

El modulo highlighter.py pertenece al editor anterior que usaba QScintilla. El que se usa actualmente es el módulo syntaxhighlighter.py

earias and others added 2 commits December 20, 2017 09:00
Elimino los dict del set_languge.py, para que mire los dict que hay en syntaxhighligher.py. Me parece mejor asi ya que queda concentrado los lenguajes que el ide maneja en un solo lugar.
@centaurialpha
Copy link
Member

centaurialpha commented Dec 21, 2017

Bien! Me gusta más ahí. @eamanu Quedaría hacer un rebase para que quede más limpio, porque se hicieron 8 commits. Y tambien resolver el conflicto que sale con la rama y ya cerraríamos la issue.

earias and others added 2 commits December 21, 2017 09:25
When a new file is saved, this file have the python file "format"
eamanu and others added 5 commits December 21, 2017 09:25
Elimino los dict del set_languge.py, para que mire los dict que hay en syntaxhighligher.py. Me parece mejor asi ya que queda concentrado los lenguajes que el ide maneja en un solo lugar.
@eamanu
Copy link
Collaborator Author

eamanu commented Dec 21, 2017

Ahí hice un rebase. Dcime si está bien.

@centaurialpha
Copy link
Member

Ahora salen 16 commits! y todavía sale como que tiene conflictos. Todavía no soy muy experto usando rebase, pero mi idea era que juntes todos tus commits y los aplastes en uno solo (squash) como para que ese sea el commit final.

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 21, 2017

Ok, se me hizo un lío. Eliminar esta rama y voy a hacerla de nuevo, asì tengas un solo commit. Tampoco se mucho de git.

@centaurialpha
Copy link
Member

@eamanu pasa que ví muchos proyectos open source en donde utilizan rebase en lugar de merge, y la verdad se ve mucho más limpio el historial de commits sin esos que te hace el merge. Igual todavía sigo estudiando eso para ver si podemos ponerlo en práctica.

@eamanu
Copy link
Collaborator Author

eamanu commented Dec 21, 2017 via email

eamanu pushed a commit to eamanu/ninja-ide that referenced this pull request Dec 21, 2017
centaurialpha pushed a commit that referenced this pull request Dec 21, 2017
@eamanu eamanu deleted the fix-2009-issue branch December 22, 2017 12:20
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