fix: four one-line bugs found in code review

- Fix pop/shift filters being swapped (pop returned first element,
  shift returned last)
- Fix ReadCollections silently dropping errors (returned nil instead
  of err)
- Fix groupPagesBy ignoring its getter argument, causing site.tags
  to contain categories instead of tags
- Fix readDataFiles stopping at first subdirectory (used break instead
  of continue), skipping data files after it alphabetically

Author: osteele

filters/filters.go

@@ -54,10 +54,10 @@ func AddJekyllFilters(e *liquid.Engine, c *config.Config) {
 		return append(array, evaluator.MustConvertItem(item, array))
 	})
 	e.RegisterFilter("pop", requireNonEmptyArray(func(array []interface{}) interface{} {
-		return array[0]
+		return array[len(array)-1]
 	}))
 	e.RegisterFilter("shift", requireNonEmptyArray(func(array []interface{}) interface{} {
-		return array[len(array)-1]
+		return array[0]
 	}))
 	e.RegisterFilter("unshift", func(array []interface{}, item interface{}) interface{} {
 		return append([]interface{}{evaluator.MustConvertItem(item, array)}, array...)

filters/filters_test.go

@@ -37,8 +37,8 @@ var filterTests = []struct{ in, expected string }{
 	{`{{ site.members | where_exp: "item", "item.name contains 'Al'" | map: "name" | join }}`, "Alonzo Alan"},
 
 	{`{{ page.tags | push: 'Spokane' | join }}`, "Seattle Tacoma Spokane"},
-	{`{{ page.tags | pop }}`, "Seattle"},
-	{`{{ page.tags | shift }}`, "Tacoma"},
+	{`{{ page.tags | pop }}`, "Tacoma"},
+	{`{{ page.tags | shift }}`, "Seattle"},
 	{`{{ page.tags | unshift: "Olympia" | join }}`, "Olympia Seattle Tacoma"},
 
 	// strings
@@ -113,6 +113,20 @@ func TestFilters(t *testing.T) {
 	}
 }
 
+func TestPopShiftFilters(t *testing.T) {
+	// pop returns the last element, shift returns the first
+	bindings := liquid.Bindings{
+		"arr": []string{"alpha", "beta", "gamma"},
+	}
+	requireTemplateRender(t, `{{ arr | pop }}`, bindings, "gamma")
+	requireTemplateRender(t, `{{ arr | shift }}`, bindings, "alpha")
+
+	// single-element array
+	single := liquid.Bindings{"arr": []string{"only"}}
+	requireTemplateRender(t, `{{ arr | pop }}`, single, "only")
+	requireTemplateRender(t, `{{ arr | shift }}`, single, "only")
+}
+
 func TestSampleFilter(t *testing.T) {
 	engine := liquid.NewEngine()
 	cfg := config.Default()

site/data.go

@@ -21,7 +21,7 @@ func (s *Site) readDataFiles() error {
 	}
 	for _, f := range files {
 		if f.IsDir() {
-			break
+			continue
 		}
 		var (
 			filename  = filepath.Join(dataDir, f.Name())

site/drop_test.go

@@ -59,6 +59,40 @@ func TestSite_ToLiquid_related_posts(t *testing.T) {
 	require.Len(t, posts, 1)
 }
 
+func TestSite_readDataFiles_skips_directories(t *testing.T) {
+	// Regression test: readDataFiles should skip directories and continue
+	// reading subsequent files (previously used break instead of continue)
+	site, err := FromDirectory("testdata/site1", config.Flags{})
+	require.NoError(t, err)
+	require.NoError(t, site.Read())
+
+	// The _data dir has: alpha.json, subdir/, zulu.json
+	// Both alpha and zulu should be loaded despite subdir/ between them
+	require.Contains(t, site.data, "alpha", "data file before directory should be loaded")
+	require.Contains(t, site.data, "zulu", "data file after directory should be loaded")
+}
+
+func TestSite_ToLiquid_tags_vs_categories(t *testing.T) {
+	drop := readTestSiteDrop(t)
+
+	// tags and categories should be distinct groupings
+	tags, ok := drop["tags"].(map[string][]Page)
+	require.True(t, ok, fmt.Sprintf("tags has type %T", drop["tags"]))
+
+	categories, ok := drop["categories"].(map[string][]Page)
+	require.True(t, ok, fmt.Sprintf("categories has type %T", drop["categories"]))
+
+	// The test post has tags: ["go", "jekyll"] and categories: ["dev"]
+	// tags should contain "go" and "jekyll" keys
+	require.Contains(t, tags, "go", "tags should contain 'go'")
+	require.Contains(t, tags, "jekyll", "tags should contain 'jekyll'")
+	require.NotContains(t, tags, "dev", "tags should not contain category 'dev'")
+
+	// categories should contain "dev" key
+	require.Contains(t, categories, "dev", "categories should contain 'dev'")
+	require.NotContains(t, categories, "go", "categories should not contain tag 'go'")
+}
+
 func TestSite_ToLiquid_static_files(t *testing.T) {
 	drop := readTestSiteDrop(t)
 	files, ok := drop["static_files"].([]*pages.StaticFile)

site/posts.go

@@ -33,7 +33,7 @@ func (s *Site) setPostVariables() {
 func (s *Site) groupPagesBy(getter func(Page) []string) map[string][]Page {
 	categories := map[string][]Page{}
 	for _, p := range s.Pages() {
-		for _, k := range p.Categories() {
+		for _, k := range getter(p) {
 			ps, found := categories[k]
 			if !found {
 				ps = []Page{}

site/read.go

@@ -151,5 +151,5 @@ func (s *Site) ReadCollections() (err error) {
 		return cols[i].Name < cols[j].Name
 	})
 	s.Collections = cols
-	return nil
+	return err
 }

site/testdata/site1/_data/alpha.json

@@ -0,0 +1 @@
+{"key": "value_a"}

site/testdata/site1/_data/zulu.json

@@ -0,0 +1 @@
+{"key": "value_z"}

site/testdata/site1/_posts/2017-07-05-post.md

@@ -1,2 +1,4 @@
 ---
+tags: ["go", "jekyll"]
+categories: ["dev"]
 ---