Skip to content

Conversation

dom96
Copy link
Contributor

@dom96 dom96 commented Jan 17, 2018

No description provided.

@Araq
Copy link
Member

Araq commented Jan 17, 2018

I don't want it in system.nim. Nor do I want people to echo types.

@bluenote10
Copy link
Contributor

bluenote10 commented Jan 18, 2018

Why is that so bad? Imho easily converting types to strings comes in handy

  • to-string conversion of generics
  • during debugging/development
  • in some meta programming use case e.g. when including type information in an error message

My most basic use case looks like this:

import typetraits

type
  Generic[T] = object
    a, b: T

proc `$`*[T](g: Generic[T]): string =
  # A meaningful string representation should encode T in some way.
  # I often want something like this for clarity:
  "Generic[" & $T & "](" & $g.a & ", " & $g.b & ")"

let g = Generic[int](a: 1, b: 2)
echo g # Generic[int](1, 2)

I just went through a bunch of my typetraits imports and they all where because of this.

@kaushalmodi
Copy link
Contributor

I also wondered why the basic foo.type.name needed importing a separate module.

For folks like me who like introspection of the code while developing (habit from Emacs lisp), this is a must have.

Knowing the type of any variable is a requirement for me to help understand Nim better. I now end up importing typetraits everywhere in my notes: https://scripter.co/notes/nim/.

@ghost
Copy link

ghost commented May 31, 2018

@kaushalmodi why do you use "intToStr" instead of "parseInt" in your notes?

@kaushalmodi
Copy link
Contributor

kaushalmodi commented May 31, 2018

@Yardanico I use both.. as they are for opposite use cases.. intToStr is for int to string conversion and parseInt is for string to int conversion.

Representing one type in another

Though I don't know if intToStr is needed at all because $ works just fine.. if foo contains 42 (an int), you can do just $foo instead of importing strutils and then doing intToStr(foo).

@ghost
Copy link

ghost commented May 31, 2018

@kaushalmodi yes, sorry, I've meant $

@ghost
Copy link

ghost commented May 31, 2018

@kaushalmodi I think intToStr is needed when you need to add leading zeroes to your number

@krux02
Copy link
Contributor

krux02 commented May 31, 2018

@Araq can you elaborate on what is so bad for this to be in system.nim? I would vote to have this in system.nim. Often when I debug things, I do echo debugging, and it is just nice when it just works on types, too. Currently I always have to look up for that package that I have to import so that echo on the type just works, and that is bothering me.

@kaushalmodi
Copy link
Contributor

@Yardanico

I think intToStr is needed when you need to add leading zeroes to your number

Let's continue this off-topic discussion here: https://forum.nim-lang.org/t/3874.

@narimiran
Copy link
Member

Based on the reactions: everybody is for this practical change, Araq is against it.

What is it gonna be? Merge (after fixing the version number in the comments) or won't fix and close?

@Araq
Copy link
Member

Araq commented Oct 22, 2018

What is it gonna be? Merge (after fixing the version number in the comments) or won't fix and close?

I surrender, can be added to system. Once/if the CIs are green, of course.

@dom96 dom96 force-pushed the typetraits-to-system branch from 9625be1 to 01f2061 Compare October 22, 2018 19:24
@dom96
Copy link
Contributor Author

dom96 commented Oct 22, 2018

Rebased, let's see if that helps.

@kaushalmodi
Copy link
Contributor

Fails because of:

�[1m�[31mFAIL: �[36mtbindtypedesc.nim C�[0m
�[1m�[36mTest "tests/metatype/tbindtypedesc.nim" in category "metatype"�[0m
�[1m�[31mFailure: reNimcCrash�[0m
�[33mExpected:�[0m

proc name*(t: typedesc): string =
## Returns the name of the given type.
##
## As of Nim v0.18.0 this is just an alias for `$` which is defined in
Copy link
Member

Choose a reason for hiding this comment

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

Before merging, change it to 0.19.2 or 0.20, depending on when this will be introduced.

Copy link
Member

Choose a reason for hiding this comment

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

The correct version is 0.20 for this.

@Araq
Copy link
Member

Araq commented Dec 12, 2018

Makes tests/metatype/tbindtypedesc.nim fail.

@timotheecour
Copy link
Member

fixed test failures, addressed comments and revived in #10071

@narimiran narimiran deleted the typetraits-to-system branch January 2, 2019 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants