Skip to content
This repository has been archived by the owner on Oct 21, 2022. It is now read-only.

Can’t eval editor.cljs in LT UI #2170

Closed
martinchooooooo opened this issue Mar 25, 2016 · 8 comments
Closed

Can’t eval editor.cljs in LT UI #2170

martinchooooooo opened this issue Mar 25, 2016 · 8 comments

Comments

@martinchooooooo
Copy link
Contributor

To reproduce

  • Have LT source in your workspace
  • open src/lt/objs/editor.cljs
  • Eval whole file (for me it’s pmeta-shift-enter)
  • Get error #< Error: This document is already in use >
  • Now when trying to use LT, nothing really works. For example try using find in the editor and you’ll get errors in the console.

Environment:

In trying to track it down..

(In the process went down the rabbit-hole trying to figure out how the whole BOT thing works with a bunch of print statements, which I’ve been wanting to do for a while so it was a bit of fun 😄)

  • The error is thrown inside of CodeMirror in function attachDoc, which we call through swapDoc in editor.cljs (source)
  • For those not particularly familiar with LT code base, like I was, the call is being made upon creating the editor object in the :init function, which calls (make) and there you have the the swapDoc call.
  • The editor object is created by editor/pool.cljs here and opener.cljs here (i think?), so perhaps this is where we have to do some kind of check to see if the editor we’re trying to create already exists?
  • Not quite sure what what CodeMirror is checking for in performing the test, but it doesn’t seem to like that the editor object we’re trying to create in LT when evaluating editor.cljs
  • If i try to eval some other file, such as pool.cljs, the eval is fine with no problem.
  • If i have pool.cljs open in current tab and editor.cljs open in another tab, but evaluate pool.cljs, then all is fine. But then go to eval editor.cljs and you’re back in trouble.
  • One sample solution I tried was to add a check if the current editor is editor.cljs, then don’t call .swapDoc.
(when (and (:doc context) (not= (:name context) "editor.cljs"))
      (.swapDoc e (-> (:doc context) deref :doc)))

but that doesn’t even open up the editor for editor.cljs in the first place, as i think this is the same code that is called when opening a tab AND when the file is eval’ed?

With all this info, does someone know what the proper cause is for this? I’m trying to track it down though I haven’t completely grok’d LT to understand what’s going on.
Maybe someone with some more CodeMirror experience maybe knows if there’s a better method than .swapDoc() that we could use or maybe is more applicable to these situations?

Thanks!

@cldwalker
Copy link
Member

Hi @martinchooooooo. Depending on the file you're re-evaling, a whole file re-eval often redefines core LT object types and recreates LT objects. This can have dire consequences in some files e.g. re-evaling object.cljs will freeze LT as LT no longer has any objects defined. In the case of editor.cljs, if you comment the editor object type and do a file re-eval, you don't see errors. My suggestion is to stick to evaling forms when exploring LT and only do a file eval as you learn more of the codebase and understand the implications of a given file eval. I can add a brief description about this in the wiki or doc/for-committers.md if you think that'd be helpful to others. Cheers

@martinchooooooo
Copy link
Contributor Author

Thanks for that info @cldwalker, that makes a lot more sense.

Funny you mention doc/for-committers.md, because it was one of the first places I looked for information on my problem. So perhaps that might be a great place to stick some info.

@carocad
Copy link

carocad commented May 18, 2016

@martinchooooooo if that solved the problem would you consider closing this issue please :) ?

@kenny-evitt
Copy link
Contributor

@carocad It's okay to keep this open; no one has added info about 'whole file evals' to the docs yet.

@rundis
Copy link
Contributor

rundis commented May 19, 2016

@cldwalker Would it make sense to provide a sprinkle of defonce in the LT codebase or would that have unfortunate side-effects ?

@cldwalker
Copy link
Member

@rundis It's unlikely but possible if applied to the wrong var e.g. one that relies on set!. If we were to introduce a convention of any file being reloadable, we should be consistent about it everywhere. Given how large our code base is and how this adds to more things we need to QA, I don't think it's worth the effort. defonce also doesn't handle docstrings so we'd have to add docstrings by mutating the var's meta which isn't pretty

@rundis
Copy link
Contributor

rundis commented May 19, 2016

mkay got it.

@kenny-evitt
Copy link
Contributor

#2254 covers the only work that remained to be done for this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants