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

Topics are (inconsistently) converted to lower-case #5222

Closed
1 of 4 tasks
michael-brade opened this issue Oct 29, 2018 · 12 comments
Closed
1 of 4 tasks

Topics are (inconsistently) converted to lower-case #5222

michael-brade opened this issue Oct 29, 2018 · 12 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@michael-brade
Copy link

  • Gitea version (or commit ref): 1.6.0-rc.1
  • Operating system: Debian unstable
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite

Description

A stream of inconsistencies regarding case-sensitivity of topics:

  • open a repository, select "Manage Topics", type "API", press Enter.
  • the topic is now converted to a lowercase "api" in a green box.
  • now click the "Done" button and watch the green box: it shows "API" again!
  • However, don't get your hopes up too soon: reload the current page (F5) and voila: it shows "api" again.

What I would expect: keep the topic as it is, don't lowercase or uppercase it.

@lunny lunny added the type/bug label Oct 30, 2018
@lafriks
Copy link
Member

lafriks commented Oct 30, 2018

It is intentionally converted to lowercase to not have problems later and also have compatibility with GitHub (it also lowercases topics)

@michael-brade
Copy link
Author

See here: #5171 (comment) and here: https://blog.gitea.io/2018/08/gitea-1.5.0-is-released/

I don't agree with "not have problems later", it is easy enough to do case-insensitive comparisons; and I don't mind being different from GitHub where it is possible to do it better than GitHub 😃 (better in this case: the user has the freedom to decide)

@lafriks
Copy link
Member

lafriks commented Oct 30, 2018

@michael-brade but than there still be problem anyway as most probably someone will decide in your place. Let's say someone in other org/repo will add topic ApI and even if you will try to add api or API it will still be converted to ApI as topics are global to whole server. I agree that there is a bug that it jumps to lowercase, than back to uppercase and than again lowercase but otherwise I think lowecase should be enforced on this.

@michael-brade
Copy link
Author

@lafriks oh! you're right, I didn't think about that... so then what about this: no conversion at all, and case-sensitive comparisons. Then everyone can do whatever they want, since API != Api. One might think that this clutters up the dropdowns, but that is a problem already present, so no real issue.

(The other possibility would be to have topics local to each organization or user, but I don't think it would have any advantage over other solutions and might really lead to trouble 😄)

@lafriks
Copy link
Member

lafriks commented Oct 30, 2018

I don't think that problem is currently present, in database all topics are currently already lowercase so it is only UI bug that needs to be fixed

@michael-brade
Copy link
Author

I think you misunderstood this one. With clutter I meant items you don't want to see in general, with typos of all sorts you can imagine. Adding uppercase letters doesn't make this kind of clutter worse. The only solution to this would be to limit suggestions to those of the current user/org, but I don't think this would be easy.

BTW: is it possible to delete a topic from the database by removing the last usage of it?

@lafriks
Copy link
Member

lafriks commented Oct 30, 2018

I don't think topics are deleted when not used any more currently. As for being case insensitive I'm against it as it will add clutter and will render them less useful. Especially taking into account if used in future in federation when we get to that.

@lunny
Copy link
Member

lunny commented Oct 31, 2018

Maybe we should add a whitelist to convert api to API and etc.

@lafriks
Copy link
Member

lafriks commented Oct 31, 2018

IMHO that is not needed as that will still not cover all

@michael-brade
Copy link
Author

Ok, I changed it for my own server to case-sensitive. And I have to say, gitea is really good code! It is very easy to find my way around in it and to understand what's going on. Lovely. If anyone is interested, here is the patch that makes Topics case-sensitive:

diff --git a/models/topic.go b/models/topic.go
index 678795a3d..69071a39b 100644
--- a/models/topic.go
+++ b/models/topic.go
@@ -21,7 +21,7 @@ func init() {
        )
 }
 
-var topicPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`)
+var topicPattern = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9-]*$`)
 
 // Topic represents a topic of repositories
 type Topic struct {
@@ -127,7 +127,7 @@ func SaveTopics(repoID int64, topicNames ...string) error {
 
                var found bool
                for _, t := range topics {
-                       if strings.EqualFold(topicName, t.Name) {
+                       if topicName == t.Name {
                                found = true
                                break
                        }
@@ -141,7 +141,7 @@ func SaveTopics(repoID int64, topicNames ...string) error {
        for _, t := range topics {
                var found bool
                for _, topicName := range topicNames {
-                       if strings.EqualFold(topicName, t.Name) {
+                       if topicName == t.Name {
                                found = true
                                break
                        }
diff --git a/public/js/index.js b/public/js/index.js
index 0bc28c4f9..5351d4040 100644
--- a/public/js/index.js
+++ b/public/js/index.js
@@ -2523,12 +2523,12 @@ function initTopicbar() {
             },
         },
         onLabelCreate: function(value) {
-            value = value.toLowerCase().trim();
+            value = value.trim();
             this.attr("data-value", value).contents().first().replaceWith(value);
             return $(this);
         },
         onAdd: function(addedValue, addedText, $addedChoice) {
-            addedValue = addedValue.toLowerCase().trim();
+            addedValue = addedValue.trim();
             $($addedChoice).attr('data-value', addedValue);
             $($addedChoice).attr('data-text', addedValue);
         }
@@ -2553,7 +2553,7 @@ function initTopicbar() {
                     rules: [
                         {
                             type: 'validateTopic',
-                            value: /^[a-z0-9][a-z0-9-]{1,35}$/,
+                            value: /^[A-Za-z0-9][A-Za-z0-9-]{1,35}$/,
                             prompt: topicPrompts.formatPrompt
                         },
                         {
diff --git a/routers/repo/topic.go b/routers/repo/topic.go
index 63fcf793f..3fa1205e7 100644
--- a/routers/repo/topic.go
+++ b/routers/repo/topic.go
@@ -30,7 +30,7 @@ func TopicsPost(ctx *context.Context) {
        invalidTopics := make([]string, 0)
        i := 0
        for _, topic := range topics {
-               topic = strings.TrimSpace(strings.ToLower(topic))
+               topic = strings.TrimSpace(topic)
                // ignore empty string
                if len(topic) > 0 {
                        topics[i] = topic

@stale
Copy link

stale bot commented Jan 5, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 5, 2019
@stale
Copy link

stale bot commented Feb 23, 2019

This issue has been automatically closed because of inactivity. You can re-open it if needed.

@stale stale bot closed this as completed Feb 23, 2019
@lunny lunny added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented and removed issue/stale labels Feb 23, 2019
@lunny lunny reopened this Feb 23, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
Development

No branches or pull requests

4 participants