Support specified config directory and XDG Base Dirs Spec #479

HaleTom · 2016-10-20T07:40:46Z

Allow setting config directory and honour XDG Base Dirs Spec

Resolve Issue #462 - "Tmuxinator config files in a different location"
Resolve Issue #360 - "XDG base directory support"
Mentioned in Issue #427 - "Introduce Analytics Module"

Allow $TMUXINATOR_CONFIG to specify a configuration directory, which
will be created (including its parent directories) if it doesn't exist. This
directory takes preference over honouring the XDG specification (below). If
specified, this will be the only configuration location used.

Honour the XDG Base Directories Specification
with regards to user specific configuration files: Look for tmuxinator
configuration files under $XDG_CONFIG_HOME/tmuxinator.

For existing setups with no $XDG_CONFIG_HOME directory, continue to
create new projects in ~/.tmuxinator for backwards compatibility.

If $XDG_CONFIG_HOME/tmuxinator exists, create new projects there.

If neither ~/.tmuxinator nor $XDG_CONFIG_HOME/tmuxinator exist, use
$XDG_CONFIG_HOME/tmuxinator for new projects. Create this directory
(including its parents) if it does not exist.

When loading a project, search $XDG_CONFIG_HOME/tmuxinator before
~/.tmuxinator. Document in comments and tests the existing behaviour
of returning only the first project file found in a recursive search of
a configuration directory.

Add Config#directories: an array of the configuration director{y,ies}
which exist. Update commands implode and list to operate upon this
array.

Add aliases for the 3 renamed methods below for backward
compatibility:

Rename Config#root -> Config#directory
This directory is the only one used when creating new project files, and
may be either $TMUXINATOR_CONFIG, $XDG_CONFIG_HOME/tmuxinator or
~/.tmuxinator.

Implementing XDG Base Dirs support with backwards compatibility means
that two directories may contain project files, making the root
nomenclature misleading as it implies a single directory.

Rename Config#project_in_root -> Config#global_project
See comment on root terminology above. global_project is also
shorter and corresponds to the naming of default_project

Rename Config#project_in_local -> Config#local_project
Fit with the naming of both global_project and default_project

Sort gem dependencies into alphabetical order

Resolve Issue tmuxinator#462 - "Tmuxinator config files in a different location" Resolve Issue tmuxinator#360 - "XDG base directory support" Mentioned in Issue tmuxinator#427 - "Introduce Analytics Module" Allow `$TMUXINATOR_CONFIG` to specify a configuration directory, which will be created if it doesn't exist. This directory takes preference over honouring the XDG specification (below). If specified, this will be the only configuration location used. Honour the [XDG Base Directories Specification] (https://specifications.freedesktop.org/basedir-spec/latest/) with regards to user specific configuration files: Look for `tmuxinator` configuration files under `$XDG_CONFIG_HOME/tmuxinator`. For existing setups with no `$XDG_CONFIG_HOME` directory, continue to create new projects in `~/.tmuxinator` for backwards compatibility. If `$XDG_CONFIG_HOME/tmuxinator` exists, create new projects there. If neither `~/.tmuxinator` nor `$XDG_CONFIG_HOME/tmuxinator` exist, use `$XDG_CONFIG_HOME/tmuxinator` for new projects. When loading a project, search `$XDG_CONFIG_HOME/tmuxinator` before `~/.tmuxinator`. Document in comments and tests the existing behaviour of returning only the first project file found in a recursive search of a configuration directory. Add Config#directories: an array of the configuration director{y,ies} which exist. Update commands `implode` and `list` to operate upon this array. Rename `Config#root` -> `Config#directory` This directory is the only one used when creating new project files, and may be either `$TMUXINATOR_CONFIG`, `$XDG_CONFIG_HOME/tmuxinator` or `~/.tmuxinator`. Implementing XDG Base Dirs support with backwards compatibility means that two directories may contain project files, making the `root` nomenclature misleading as it implies a single directory. Rename `Config#project_in_root` -> `Config#global_project` See comment on `root` terminology above. `global_project` is also shorter and corresponds to the naming of `default_project` Rename `Config#project_in_local` -> `Config#local_project` Fit with the naming of both `global_project` and `default_project`

Allow existing code to work with renamed methods Deprecated methods: ignore the 1st, use the 2nd alias :root :directory alias :project_in_root :global_project alias :project_in_local :local_project

coveralls · 2016-10-20T07:55:51Z

Coverage decreased (-1.3%) to 97.934% when pulling 93f5842 on HaleTom:Issues-360-462 into 6df10d1 on tmuxinator:master.

coveralls · 2016-11-08T09:32:57Z

Coverage increased (+0.05%) to 99.241% when pulling eba142f on HaleTom:Issues-360-462 into 6df10d1 on tmuxinator:master.

coveralls · 2016-11-08T10:27:59Z

Coverage increased (+0.05%) to 99.241% when pulling 9b5e2ca on HaleTom:Issues-360-462 into 6df10d1 on tmuxinator:master.

HaleTom · 2016-11-08T10:39:03Z

OK, ready for comments / merge.

adamstrickland · 2016-11-08T17:04:18Z

This looks good 👍

adamstrickland · 2016-11-16T18:21:10Z

@ethagnawl @J3RN do you have any feedback on this for @HaleTom? I say LGTM.

ethagnawl · 2016-11-17T03:08:27Z

@adamstrickland I've reviewed these changes a few times and, barring a few small nitpicks (adding comments now), everything looks good to me. Though, I haven't had a chance to pull it down and experiment yet.

ethagnawl · 2016-11-17T03:11:06Z

lib/tmuxinator/config.rb

-        root_dir
+      # The directory (created if needed) in which to store new projects
+      def directory
+        environment = ENV["TMUXINATOR_CONFIG"]


This config get/test behavior is used in multiple places, so it might warrant its own helper method.

Thanks for the thorough review, I'll get on to these in a couple of days to allow for more reviewers to have their say in the meantime.

ethagnawl · 2016-11-17T03:12:33Z

lib/tmuxinator/config.rb

-        Dir["#{Tmuxinator::Config.root}/**/*.yml"].sort.map do |path|
-          path.gsub("#{Tmuxinator::Config.root}/", "").gsub(".yml", "")
+        configs = []
+        directories.each do |directory|


Is there a good reason not to use map here?

I guess a brain-fart doesn't count :)

coveralls · 2016-11-21T09:59:51Z

Coverage increased (+0.05%) to 99.24% when pulling 0c0f20c on HaleTom:Issues-360-462 into 6df10d1 on tmuxinator:master.

HaleTom · 2016-11-21T10:35:28Z

@ethagnawl I trust that this is good to go now. Thanks for the thorough review - I learned from it.

J3RN · 2016-11-21T17:17:07Z

This LGTM 👍

ethagnawl · 2016-12-06T20:59:48Z

@HaleTom I had some time to experiment with this today. With the exception of one very small item (covered below), everything appears to work as intended.

I noticed that the config path at the top of the project yml file always uses the old default directory (~/.tmuxinator/$PROJECT). It's possible that this is caused by some legacy config on my system and it doesn't break anything (that I'm aware of, at least) but it's probably worth addressing. Also, as a bonus, the fix will likely address #440.

ethagnawl · 2017-01-29T16:27:13Z

@Soliah @adamstrickland Any thoughts on the above? Shall we merge this as-is? It might be a bit self-serving, but I'd like to begin using XDG on my dev machines.

ethagnawl · 2017-02-04T22:44:06Z

Merging this broke the build. I'm looking into it now and will submit a patch.

@HaleTom If you're available, would you mind taking a look to see if anything jumps out at you?

UPDATE:

I wasn't/still am not seeing this failure locally.
I've reverted the merge, but still have yet to figure out what the underlying problem is.
Does anyone know if it's possible to re-open the PR? If not, perhaps we could ask @HaleTom to add a new commit and submit a new PR.

HaleTom · 2017-02-07T07:32:23Z

I'm working towards diagnosing this. I got out of hospital yesterday (dengue), and am battling a json error when running bundle install under Manjaro, but I am trucking on...

HaleTom · 2017-02-07T09:15:10Z

The doco for Dir::glob says:

Note that case sensitivity depends on your system (so File::FNM_CASEFOLD is ignored), as does the order in which the results are returned.

I'll append .sort so that the results are (hopefully) system-independent.

HaleTom · 2017-02-07T10:05:56Z

PR #506 created. I hope this is what you were after - let me know if not.

ethagnawl · 2017-02-07T18:46:46Z

@HaleTom Wow. I hope you're feeling better! :|

Thanks for looking into this issue. Unfortunately, PR #506 doesn't look like it's going to address the problem and reintroduce the changes from your original PR. Perhaps that's because #506 wants to merge its changes into the branch: tmuxinator:revert-479-Issues-360-462? Could you try creating a new PR and selecting tmuxinator/tmuxinator base:master as the target branch?

UPDATE:

I don't think that approach is going to work. I'll investigate further and follow-up if/when I find a workable solution.
Please see Attempt at salvaging PR #479 #507.

HaleTom added 3 commits October 20, 2016 14:37

Sort dependency lists

7a85a44

Add aliases for deprecated methods

93f5842

Allow existing code to work with renamed methods Deprecated methods: ignore the 1st, use the 2nd alias :root :directory alias :project_in_root :global_project alias :project_in_local :local_project

HaleTom added 4 commits November 8, 2016 14:20

Add failing test with non-existant parent

a69ee43

Refactor Config#project

3e3b1bc

Fix test and code

bfe338b

Create parent dirs in paths if they don't exist

eba142f

Silence rubocop

9b5e2ca

Save 2 lines

f86a065

HaleTom mentioned this pull request Nov 16, 2016

XDG base directory support #360

Closed

ethagnawl reviewed Nov 17, 2016

View reviewed changes

HaleTom added 2 commits November 21, 2016 16:35

Use map instead of +=

69c9cb7

Add helper environment for ENV["TMUXINATOR_CONFIG"]

0c0f20c

HaleTom mentioned this pull request Nov 21, 2016

Add support for the XDG Base Directory Specification tmux/tmux#142

Closed

ghost self-requested a review January 29, 2017 22:27

ghost approved these changes Jan 29, 2017

View reviewed changes

Merge branch 'master' into Issues-360-462

f128ec4

ethagnawl merged commit af90ade into tmuxinator:master Feb 4, 2017

ethagnawl mentioned this pull request Feb 4, 2017

Revert "Support specified config directory and XDG Base Dirs Spec" #505

Merged

HaleTom mentioned this pull request Feb 7, 2017

Sort output of Dir.glob for consistency #506

Closed

ethagnawl mentioned this pull request Feb 7, 2017

Attempt at salvaging PR #479 #507

Merged

HaleTom mentioned this pull request Mar 2, 2017

Support specified config directory and XDG Base Dirs Spec (redux) #511

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support specified config directory and XDG Base Dirs Spec #479

Support specified config directory and XDG Base Dirs Spec #479

HaleTom commented Oct 20, 2016 •

edited

Loading

coveralls commented Oct 20, 2016 •

edited

Loading

coveralls commented Nov 8, 2016 •

edited

Loading

coveralls commented Nov 8, 2016

HaleTom commented Nov 8, 2016

adamstrickland commented Nov 8, 2016

adamstrickland commented Nov 16, 2016

ethagnawl commented Nov 17, 2016

ethagnawl Nov 17, 2016

HaleTom Nov 17, 2016

ethagnawl Nov 17, 2016

HaleTom Nov 17, 2016

coveralls commented Nov 21, 2016

HaleTom commented Nov 21, 2016

J3RN commented Nov 21, 2016

ethagnawl commented Dec 6, 2016

ethagnawl commented Jan 29, 2017

ethagnawl commented Feb 4, 2017 •

edited

Loading

HaleTom commented Feb 7, 2017

HaleTom commented Feb 7, 2017

HaleTom commented Feb 7, 2017 •

edited

Loading

ethagnawl commented Feb 7, 2017 •

edited

Loading

Support specified config directory and XDG Base Dirs Spec #479

Support specified config directory and XDG Base Dirs Spec #479

Conversation

HaleTom commented Oct 20, 2016 • edited Loading

coveralls commented Oct 20, 2016 • edited Loading

coveralls commented Nov 8, 2016 • edited Loading

coveralls commented Nov 8, 2016

HaleTom commented Nov 8, 2016

adamstrickland commented Nov 8, 2016

adamstrickland commented Nov 16, 2016

ethagnawl commented Nov 17, 2016

ethagnawl Nov 17, 2016

Choose a reason for hiding this comment

HaleTom Nov 17, 2016

Choose a reason for hiding this comment

ethagnawl Nov 17, 2016

Choose a reason for hiding this comment

HaleTom Nov 17, 2016

Choose a reason for hiding this comment

coveralls commented Nov 21, 2016

HaleTom commented Nov 21, 2016

J3RN commented Nov 21, 2016

ethagnawl commented Dec 6, 2016

ethagnawl commented Jan 29, 2017

ethagnawl commented Feb 4, 2017 • edited Loading

HaleTom commented Feb 7, 2017

HaleTom commented Feb 7, 2017

HaleTom commented Feb 7, 2017 • edited Loading

ethagnawl commented Feb 7, 2017 • edited Loading

HaleTom commented Oct 20, 2016 •

edited

Loading

coveralls commented Oct 20, 2016 •

edited

Loading

coveralls commented Nov 8, 2016 •

edited

Loading

ethagnawl commented Feb 4, 2017 •

edited

Loading

HaleTom commented Feb 7, 2017 •

edited

Loading

ethagnawl commented Feb 7, 2017 •

edited

Loading