tools.namespace

c.t.n.move: does not support to move .cljc files

Details

  • Type: Enhancement Enhancement
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Patch:
    Code

Description

it seems that c.t.n.move did not get any reader conditionals love. I use this namespace to power mranderson: a source inlining tool used by cider-nrepl and refactor-nrepl in order to avoid clashes with the code/projects their users work on. lately we started adding dependencies to refactor-nrepl (well, tools.namespace itself) which may use .cljc files. source inlining started failing for these source files.

Activity

Hide
Stuart Sierra added a comment -

Good idea - glad to see how people are actually using tools.namespace in other dev cools.

Could this be extended further to take 'platform' as a parameter, as in c.t.n.find or c.t.n.dir?

(I deliberately excluded c.t.n.move to limit the scope of changes in 0.3.0, but your patch demonstrates that it may not not be such a difficult change to make.)

Show
Stuart Sierra added a comment - Good idea - glad to see how people are actually using tools.namespace in other dev cools. Could this be extended further to take 'platform' as a parameter, as in c.t.n.find or c.t.n.dir? (I deliberately excluded c.t.n.move to limit the scope of changes in 0.3.0, but your patch demonstrates that it may not not be such a difficult change to make.)
Hide
Benedek Fazekas added a comment -

first patch does what you asked for I hope. Kept original signiture of move-ns for backward compatibility: clj platform used by default.

second patch is pushing this a bit further: if platform is not provided it tries to guess it based on the file's extension of the old-sym. you might see this as over complication: feel free to pick the first patch then.

Show
Benedek Fazekas added a comment - first patch does what you asked for I hope. Kept original signiture of move-ns for backward compatibility: clj platform used by default. second patch is pushing this a bit further: if platform is not provided it tries to guess it based on the file's extension of the old-sym. you might see this as over complication: feel free to pick the first patch then.
Hide
Stuart Sierra added a comment - - edited

Thanks for working on these patches. I did some more testing, and uncovered some more complications.

Guessing the file extension leads to unpredictable results, because you don't know which file gets moved. It's not uncommon to have both .clj and .cljs version of the same namespace, for example with :require-macros in ClojureScript.

Unfortunately, even specifying a "platform" argument doesn't completely solve the problem, because you can have .cljc file at the same time as .clj and .cljs. This may not be common, but it's explicitly allowed for in the design of reader conditionals. For example, a library may have a platform-neutral .cljc file and override it with a platform-specific file. In this case, even if you specify a "platform" argument, you still can't control which file gets moved.

So now I think the only way for an operation like "move namespace" to make sense is to move all the files defining that namespace, preserving their file extensions. This seems like what one would want from a user perspective as well.

So move-ns should take new/old symbols, find all the matching files with any extension, move/rename them all while preserving extensions, then apply the textual transformation to all .clj, .cljs, and .cljc files.

If we're searching for files, we might as well eliminate the need to specify the source path, so this also relates to TNS-39.

Show
Stuart Sierra added a comment - - edited Thanks for working on these patches. I did some more testing, and uncovered some more complications. Guessing the file extension leads to unpredictable results, because you don't know which file gets moved. It's not uncommon to have both .clj and .cljs version of the same namespace, for example with :require-macros in ClojureScript. Unfortunately, even specifying a "platform" argument doesn't completely solve the problem, because you can have .cljc file at the same time as .clj and .cljs. This may not be common, but it's explicitly allowed for in the design of reader conditionals. For example, a library may have a platform-neutral .cljc file and override it with a platform-specific file. In this case, even if you specify a "platform" argument, you still can't control which file gets moved. So now I think the only way for an operation like "move namespace" to make sense is to move all the files defining that namespace, preserving their file extensions. This seems like what one would want from a user perspective as well. So move-ns should take new/old symbols, find all the matching files with any extension, move/rename them all while preserving extensions, then apply the textual transformation to all .clj, .cljs, and .cljc files. If we're searching for files, we might as well eliminate the need to specify the source path, so this also relates to TNS-39.
Hide
Benedek Fazekas added a comment -

I have not abandoned this ticket or the patches but it seems it is related to a bigger problem in terms of supporting multiplatform projects therefore related to TNS-38 too.

I am also contemplating the wider context: `move-ns` is not only used in mranderson for source inlining but there is a refactor-nrepl feature (rename file or dir) which basically reimplements this functionality using other parts of TNS.

The mranderson story for me is about exploring different ways of dependency handling really, there was a good lightning talk by Glen Mailer summarising the problem on clojureX this year. My ideas are along creating local, deeply nested dependencies and perhaps enhance the ns macro to be able to load them – not even sure that is feasible. This would eliminate the need for source inlining and more importantly would be much more hygienic approach I think.

Hope this makes sense. Sorry for the brain dump which is obviously exceeds the scope of this ticket.

Show
Benedek Fazekas added a comment - I have not abandoned this ticket or the patches but it seems it is related to a bigger problem in terms of supporting multiplatform projects therefore related to TNS-38 too. I am also contemplating the wider context: `move-ns` is not only used in mranderson for source inlining but there is a refactor-nrepl feature (rename file or dir) which basically reimplements this functionality using other parts of TNS. The mranderson story for me is about exploring different ways of dependency handling really, there was a good lightning talk by Glen Mailer summarising the problem on clojureX this year. My ideas are along creating local, deeply nested dependencies and perhaps enhance the ns macro to be able to load them – not even sure that is feasible. This would eliminate the need for source inlining and more importantly would be much more hygienic approach I think. Hope this makes sense. Sorry for the brain dump which is obviously exceeds the scope of this ticket.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated: