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

load project-specific configuration as well as user-specific #1469

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/yard/cli/yri.rb
Expand Up @@ -5,14 +5,14 @@ module YARD
module CLI
# A tool to view documentation in the console like `ri`
class YRI < Command
# The location in {YARD::CONFIG_DIR} where the YRI cache file is loaded
# The location in {YARD::USER_CONFIG_DIR} where the YRI cache file is loaded
# from.
CACHE_FILE = File.expand_path(File.join(YARD::Config::CONFIG_DIR, 'yri_cache'))
CACHE_FILE = File.expand_path(File.join(YARD::Config::USER_CONFIG_DIR, 'yri_cache'))

# A file containing all paths, delimited by newlines, to search for
# yardoc databases.
# @since 0.5.1
SEARCH_PATHS_FILE = File.expand_path(File.join(YARD::Config::CONFIG_DIR, 'yri_search_paths'))
SEARCH_PATHS_FILE = File.expand_path(File.join(YARD::Config::USER_CONFIG_DIR, 'yri_search_paths'))

# Default search paths that should be loaded dynamically into YRI. These paths
# take precedence over all other paths ({SEARCH_PATHS_FILE} and RubyGems
Expand Down
53 changes: 37 additions & 16 deletions lib/yard/config.rb
Expand Up @@ -92,14 +92,17 @@ class << self
end

# The location where YARD stores user-specific settings
CONFIG_DIR = File.expand_path('~/.yard')
USER_CONFIG_DIR = File.expand_path('~/.yard')

# The main configuration YAML file.
CONFIG_FILE = File.join(CONFIG_DIR, 'config')
# @deprecated Use {USER_CONFIG_DIR}
CONFIG_DIR = USER_CONFIG_DIR
Comment on lines +95 to +98
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure this is a necessary deprecation-- it seems arbitrary to name this USER_CONFIG_DIR when there does not seem to be a "project config directory" equivalent anyway.


# @deprecated Use {config_file}, passing the necessary directory
CONFIG_FILE = File.join(USER_CONFIG_DIR, 'config')

# File listing all ignored plugins
# @deprecated Set `ignored_plugins` in the {CONFIG_FILE} instead.
IGNORED_PLUGINS = File.join(CONFIG_DIR, 'ignored_plugins')
IGNORED_PLUGINS = File.join(USER_CONFIG_DIR, 'ignored_plugins')
Copy link
Owner

Choose a reason for hiding this comment

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

It seems as though IGNORED_PLUGINS would not support project configuration? Seems like an omission in functionality.


# Default configuration options
DEFAULT_CONFIG_OPTIONS = {
Expand All @@ -113,19 +116,36 @@ class << self
# to allow it to be used as a plugin.
YARD_PLUGIN_PREFIX = /^yard[-_]/

# Loads settings from {CONFIG_FILE}. This method is called by YARD at
# load time and should not be called by the user.
# @return [void]
def self.load
self.options = SymbolHash.new(false)
options.update(DEFAULT_CONFIG_OPTIONS)
options.update(read_config_file)
load_project_config_file
load_user_config_file
load_commandline_safemode
add_ignored_plugins_file
translate_plugin_names
load_plugins
end

def self.config_file(dir)
File.join(dir, 'config')
end

def self.load_project_config_file
load_config_file(config_file(".yard"))
end

def self.load_user_config_file
load_config_file(config_file(USER_CONFIG_DIR))
end
Comment on lines +130 to +140
Copy link
Owner

Choose a reason for hiding this comment

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

These should be documented (and whichever are not intended to be used should be marked @private).


# Loads settings from `config_file`. This method is called by YARD at
# load time and should not be called by the user.
# @return [void]
def self.load_config_file(config_file)
options.update(read_config_file(config_file))
rescue => e
log.error "Invalid configuration file, using default options."
log.error "Invalid configuration file #{config_file}, using default options."
log.backtrace(e)
options.update(DEFAULT_CONFIG_OPTIONS)
end
Expand All @@ -134,8 +154,8 @@ def self.load
# @return [void]
def self.save
require 'yaml'
Dir.mkdir(CONFIG_DIR) unless File.directory?(CONFIG_DIR)
File.open(CONFIG_FILE, 'w') {|f| f.write(YAML.dump(options)) }
Dir.mkdir(USER_CONFIG_DIR) unless File.directory?(USER_CONFIG_DIR)
File.open(config_file(USER_CONFIG_DIR), 'w') {|f| f.write(YAML.dump(options)) }
end

# Loads gems that match the name 'yard-*' (recommended) or 'yard_*' except
Expand Down Expand Up @@ -231,15 +251,16 @@ def self.translate_plugin_names
end

# Loads the YAML configuration file into memory
# @param [String] config_file the file to load
# @return [Hash] the contents of the YAML file from disk
# @see CONFIG_FILE
def self.read_config_file
if File.file?(CONFIG_FILE)
# @see config_file
def self.read_config_file(config_file)
Copy link
Owner

Choose a reason for hiding this comment

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

Requiring a parameter would be a breaking change. Why not default to using CONFIG_FILE?

if File.file?(config_file)
require 'yaml'
if YAML.respond_to?(:safe_load_file)
YAML.safe_load_file(CONFIG_FILE, permitted_classes: [SymbolHash, Symbol])
YAML.safe_load_file(config_file, permitted_classes: [SymbolHash, Symbol])
else
YAML.load_file(CONFIG_FILE)
YAML.load_file(config_file)
end
else
{}
Expand Down
2 changes: 1 addition & 1 deletion lib/yard/registry.rb
Expand Up @@ -31,7 +31,7 @@ module YARD
# Registry.resolve(P('YARD::CodeObjects::Base'), '#docstring', true)
module Registry
DEFAULT_YARDOC_FILE = ".yardoc"
LOCAL_YARDOC_INDEX = File.expand_path(File.join(Config::CONFIG_DIR, 'gem_index'))
LOCAL_YARDOC_INDEX = File.expand_path(File.join(Config::USER_CONFIG_DIR, 'gem_index'))
DEFAULT_PO_DIR = "po"

extend Enumerable
Expand Down
5 changes: 5 additions & 0 deletions spec/config_spec.rb
Expand Up @@ -2,6 +2,11 @@
require 'yaml'

RSpec.describe YARD::Config do
before do
allow(File).to receive(:file?).with(".yard/config").and_return(false)
allow(File).to receive(:file?).with(".yardopts").and_return(false)
end

Comment on lines +5 to +9
Copy link
Owner

Choose a reason for hiding this comment

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

I would expect to see actual tests for the new APIs:

  1. load_config_file is missing tests.
  2. read_config_file behavior seems to be missing tests.

Some of this may already be tested via wider unit tests, but there should at least be a few scenarios for the theoretical project based usage.

describe ".load" do
before do
expect(File).to receive(:file?).twice.with(CLI::Yardoc::DEFAULT_YARDOPTS_FILE).and_return(false)
Expand Down