bmag / emacs-purpose

Manage Windows and Buffers According to Purposes
GNU General Public License v3.0
498 stars 23 forks source link

follow up for the ticket https://github.com/bmag/emacs-purpose/issues/106 #107

Open sergeyglazyrindev opened 7 years ago

sergeyglazyrindev commented 7 years ago

Hey bmag!

I moved out almost all not necessary stuff from window-purpose-x.el though there's one problem: maybe you can advice (I am debugging it now - maybe there's a problem with my init.el)

Hm, I removed everything from my init.el except window-purpose initialization... and still there's a problem: if I fix default layout in your window-purpose-x.el then it works, if I put the same layout to separated layout file it doesn't work... Any idea ?

I included changes to layout, so you may play with these changes.....

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 67.53% when pulling a5fa09c015c5affea29e8fc6c85d6f7e6ffcec0f on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

sergeyglazyrindev commented 7 years ago

And yes, I'll cleanup this pull request to mimic current main project state once we solve issue with layout which doesn't work properly being in separated file. Sorry.. for some wrong changes here.

bmag commented 7 years ago

Thanks, I reviewed the PR and submitted feedback.

As I suggested in the issue report, please implement the change as a new extension, and don't change code1 itself. Copy code1 and rename code1 to code2, then do the changes you want to code2. If I'm not clear feel free to ask questions.

What exactly is "the issue with layout which doesn't work properly being in separated file"?

sergeyglazyrindev commented 7 years ago

Hi bmag! I've got an idea why my layout doesn't work in separated file... Let me try out one idea and then if that doesn't help, I'll share details about the problem. Thanks for review. I am not an expert in elisp :( And I appreciate your suggestions.

sergeyglazyrindev commented 7 years ago

Oh yes, sorry forgot to ask you: what do you mean by "add code2" instead of "changing code1" ? Are you talking about adding separated vars, etc with code2 prefix like you named variable "purpose-x-code1--window-layout" ?

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 67.53% when pulling 8c179f0792042092bfbe43e21f60c5efcf8f066c on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 67.53% when pulling aba22525a45d106c9ab180990553bfbfde23368d on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 67.53% when pulling acb140441d047f1baa98c56be322a1de1f981dcf on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

sergeyglazyrindev commented 7 years ago

Hey bmag! Please check this out now! I solved the issue with setting up my layout though I had to get rid of purpose-x-code1-setup function calling, instead I added my own initialization:

(defun load-purpose-mode ()
  (interactive)
  (purpose-x-code1--setup-ibuffer)
  (purpose-x-code1-update-dired)
  (ignore-errors (imenu-list-minor-mode))
  (frame-or-buffer-changed-p 'purpose-x-code1-buffers-changed)
  (add-hook 'post-command-hook #'purpose-x-code1-update-changed)
  (purpose-load-window-layout-file "~/.emacs.d/layouts/full-ide.window-layout")
  (todo-mode-get-buffer-create)
)
(global-set-key (kbd "M-L") 'load-purpose-mode)

That works fine now. And I added my layout into your folder "layouts" I am thinking, how do I describe the way how to setup it properly ? Because I like todo purpose buffer and it helps much to find all todos and handle it. Also I like the idea of misc buffer. Though I listed all needed major mode and its modes through variable purpose-user-mode-purposes but I think how to simplify the setup for another users. Would you mind if I add a part how to setup the things to your README ? Or is it better to write a full-ide-README.md in layouts folder ?

bmag commented 7 years ago

Oh yes, sorry forgot to ask you: what do you mean by "add code2" instead of "changing code1" ? Are you talking about adding separated vars, etc with code2 prefix like you named variable "purpose-x-code1--window-layout" ?

Yes, I'm talking about adding separate variables and functions. Right now, these exist:

(defvar purpose-x-code1--window-layout ...)
(defvar purpose-x-code1-purpose-config ...)
(defvar purpose-x-code1-buffers-changed ...)
(define-ibuffer-filter purpose-x-code1-ibuffer-files-only ...)
(defun purpose-x-code1--setup-ibuffer ...)
(defun purpose-x-code1--unset-ibuffer ...)
(defun purpose-x-code1-update-dired ...)
(defun purpose-x-code1-update-changed ...)
(defun purpose-x-code1-setup ...)
(defun purpose-x-code1-unset ...)

What I'm saying is to add below them:

(defvar purpose-x-code2--window-layout ...)
(defvar purpose-x-code2-purpose-config ...)
(defun purpose-x-code2-update-dired ...)
(defun purpose-x-code2-update-changed ...)
(defun purpose-x-code2-setup ...)
(defun purpose-x-code2-unset ...)

I think you can reuse purpose-x-code1-buffers-changed, purpose-x-code1--setup-ibuffer and purpose-x-code1--unset-ibuffer in code2 without copying them.

And I added my layout into your folder "layouts" I am thinking, how do I describe the way how to setup it properly ? Because I like todo purpose buffer and it helps much to find all todos and handle it. Also I like the idea of misc buffer.

I prefer the layout to be stored in purpose-x-code2--window-layout, rather than as a separate layout file. Ideally code2 will just work by calling purpose-x-code2-setup, including setting up the layout, and won't need a big description. Any necessary description can be included in the docstring of purpose-x-code2-setup or in the wiki.

Though I listed all needed major mode and its modes through variable purpose-user-mode-purposes but I think how to simplify the setup for another users. Would you mind if I add a part how to setup the things to your README ? Or is it better to write a full-ide-README.md in layouts folder ?

I prefer the necessary configuration to be stored in purpose-x-code2-purpose-config, so the user doesn't need to do anything more than calling purpose-x-code2-setup. That said, the todo.org buffer is custom-made for your workflow, right? It isn't a standard thing? So if the user uses a different buffer for viewing todos, he/she will have to assign a todo purpose to this buffer. This kind of info can go in the wiki as well. Instructions on how to create a todo.org can also go in the wiki.

I solved the issue with setting up my layout though I had to get rid of purpose-x-code1-setup function calling, instead I added my own initialization

It's weird that you need to wrap imenu-list-minor-mode inside an ignore-errors block. The index can't be created when imenu-list-minor-mode is invoked from an indexable buffer (e.g. the *scratch* buffer if activated at Emacs startup), but imenu-list is supposed to catch these errors and handle them gracefully. Are you using an old version of imenu-list.el by any chance?

sergeyglazyrindev commented 7 years ago

Ok, now it's clear I'll do that Thank you for suggestions! But I think I'll find a time a the end of the week. Now busy at work.

bmag commented 7 years ago

Any news?

sergeyglazyrindev commented 7 years ago

Hi, I scheduled it but last weeks very crazy. Will do that asap. Sorry :( for delay.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling a13d6cb1b7d8099298be9ad2be73f9d96b0762c6 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling f21238efac67727cbf10415f8d6d44d70fcd605f on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling f21238efac67727cbf10415f8d6d44d70fcd605f on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling 53341eb845ae78d23cbc2ee56cfdbe5fd7ced6bf on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.4%) to 66.183% when pulling 53341eb845ae78d23cbc2ee56cfdbe5fd7ced6bf on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling ab99409877fa95bbecca531db7af53b1c0e95708 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling 8b74770ea9c2397ed655dddb118672e6c97f0de8 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.4%) to 67.217% when pulling 8b74770ea9c2397ed655dddb118672e6c97f0de8 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

sergeyglazyrindev commented 6 years ago

Hello dear bmag. Sorry for the huge delay I had problems with health and couldn't work enough to do anything in open source world. Now I restored and added changes you requested + added a documentation how to install things and use full-ide.window-layout. Please let me know your thoughts

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling 9ad678adf1b6975b485d1360fab2eeff32096836 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling 91f095645a3a7f05ffab911cbe9a0a75a6079c8e on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling d5cc4151a8668571d8a5c567a8e1d72161ae8106 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling b710f681981bef9f5a285385104118c93ddb01b4 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling b69ffadb53fe7aa806d7a7f14d7ea07701761a6c on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling b69ffadb53fe7aa806d7a7f14d7ea07701761a6c on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.9%) to 65.689% when pulling 383dd1eb57a13849e439c0030a0ce65243cde089 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-1.8%) to 65.786% when pulling 5a97da934ac2065f7852afa113bde210d14771d5 on sergeyglazyrindev:master into 67ecaa2b52c113f92913c3beb9fb7f302bd50318 on bmag:master.

bmag commented 6 years ago

@sergeyglazyrindev It's ok, I understand, hope you're all better now and wish you the best.

As for this PR, I'm very busy myself this month, so it will take me some time to review these. Worst case scenario, I will review them towards the end of February.

bmag commented 6 years ago

Hey, sorry for the late response, I was really busy and turns out "end of February" wasn't the worst case :frowning_face:

I know putting together this PR is a considerable amount of work, and I appreciate the effort you put into it, but I feel it ended up too big and out-of-scope for Purpose itself, so I won't be merging it. However, I'd like to convert it to a new page in Purpose's wiki. Would you mind if I did that?