nim-lang / fusion

Fusion is for now an idea about how to grow Nim's ecosystem without the pain points of more traditional approaches.
MIT License
128 stars 16 forks source link

Add terminalutils #19

Closed juancarlospaco closed 3 years ago

juancarlospaco commented 4 years ago

So more than once been asked "How to import the Nimble menu thingy" or if its possible to do so, so I tough lets make it happen, and stolen the code from Nimble, is the same code basically, code is already there anyways..., just making it more usable.

:)

juancarlospaco commented 4 years ago

Do you prefer other module name?, like tui or something?, just in case some day something can be added to it. :thinking:

ghost commented 4 years ago

This is the version with my small styling/functionality changes just for reference:

proc promptInteractive*(question: string, answers: openArray[string], width: Positive = 80): string =
  ## Terminal prompt that asks a `question` and returns only one of the answers from possible `answers`.
  ##
  ## .. code-block:: Nim
  ##   echo promptInteractive("is Schrödinger's Cat alive?", ["yes", "no", "maybe"])
  ##
  # Adapted from Nimble source code to stdlib, adding width optional argument.
  assert question.len > 0, "Question must not be empty"
  assert answers.len > 0, "There must be at least one possible answer"
  writeStyled(question, {styleBright})
  var
    current = 0
    selected = false
  # Incase the cursor is at the bottom of the terminal
  for arg in answers: stdout.write "\n"
  # Reset the cursor to the start of the selection prompt
  cursorUp(stdout, answers.len)
  cursorForward(stdout, width)
  hideCursor(stdout)

  # The selection loop
  while not selected:
    setForegroundColor(fgDefault)
    # Loop through the options
    for i, arg in answers:
      # Check if the option is the current
      if i == current:
        writeStyled("> " & arg & " <", {styleBright, styleUnderscore})
      else:
        writeStyled("  " & arg & "  ", {styleDim})
      # Move the cursor back to the start
      cursorBackward(stdout, arg.len + 4)
      # Move down for the next item
      cursorDown(stdout)
    # Move the cursor back up to the start of the selection prompt
    cursorUp(stdout, answers.len)
    resetAttributes(stdout)

    # Ensure that the screen is updated before input
    flushFile(stdout)
    # Begin key input
    while true:
      case getch():
      of '\t', '\x1B':
        current = (current + 1) mod answers.len
        break
      of '\r', ' ':
        selected = true
        break
      of '\3':
        showCursor(stdout)
        raise newException(OSError, "Keyboard interrupt")
      else: discard

  # Erase all lines of the selection
  for i in 0 ..< answers.len:
    eraseLine(stdout)
    cursorDown(stdout)
  # Move the cursor back up to the initial selection line
  cursorUp(stdout, answers.len)
  showCursor(stdout)
  result = answers[current]

I've found some problems with the implementation: 1) Default width is IMO too high, it should probably be calculated as a width of the longest answer + length of the question + 2 2) Maybe it's better to put possible answers on the next line? Then we'll have less width usage since the question could be long, but the answers will be short

timotheecour commented 4 years ago

can this be customized to use a horizontal vs vertical layout?

ghost commented 4 years ago

@timotheecour you mean questions on a single line vs questions on separate lines?

timotheecour commented 4 years ago

no I mean answers layout:

yes no maybe

vs

yes no maybe

bitnom commented 4 years ago

Was mentioned here

thx for efforts

juancarlospaco commented 3 years ago

Ready to merge.

timotheecour commented 3 years ago

@juancarlospaco can you assign the review to @dom96 instead because of the code history?

juancarlospaco commented 3 years ago

@timotheecour No; I do not have permission to do it.

timotheecour commented 3 years ago

done

dom96 commented 3 years ago

So more than once been asked "How to import the Nimble menu thingy" or if its possible to do so, so I tough lets make it happen, and stolen the code from Nimble, is the same code basically, code is already there anyways..., just making it more usable.

There is a reason the Nimble code is available as a library, why not just use it?

nc-x commented 3 years ago

Firstly, I would like to say that i am one of the few people who still do not understand the reasoning behind fusion, and the criteria by which a module is supposed to be put into stdlib, or fusion, or a separate nimble package.

Personally, I would have preferred no fusion, and some of the modules from here should have been in stdlib and some as separate nimble packages, so I won't comment on whether we need this terminalutils in fusion or not.

However,

There is a reason the Nimble code is available as a library, why not just use it?

(I don't know how much code is included in the nimble as a library, so i may be wrong but anyways...)

Because nimble is not a tui library? And if I need simple tui utilities I would prefer importing a simple stdlib module, or a tui library from nimble, and not import a fullblown package manager just to use 5% of its functionality, especially considering the fact that the functionality I want to use has nothing to do with package management?

dom96 commented 3 years ago

@nc-x nothing stops you from just importing the cli module from Nimble. Just import nimblepkg/cli and you can use it. It's a pretty lightweight module: https://github.com/nim-lang/nimble/blob/master/src/nimblepkg/cli.nim

nc-x commented 3 years ago

Is the cli module a separate nimble package, or does it come inside the nimble package?

the download size of nimble won't be particularly big, but still (atleast to me) it doesn't feel right to add dependency to nimble for using tui utilities.

In any case, we also need to take into account the discoverability of nimble as a tui library. Even if nimble directory shows nimble when searching for tui library, most people are going to ignore it thinking that maybe it is tagged wrong because nimble is a package manager not tui library.

timotheecour commented 3 years ago

it doesn't feel right to add dependency to nimble for using tui utilities

I agree. And even though nimble is, well, a package, manager, it would still be possible for it to have a require tui optional dependency (not sure if feasible at the moment, but it's doable). Libraries grow and having a dedicated tui package makes more sense than having to rely on nimble, as it'd quickly enter a friction mode (separation of concerns: nimble shouldn't focus on full blown tui). So I support having tui separately from nimble instead of requiring a dependency on nimble.

one of the few people who still do not understand the reasoning behind fusion, and the criteria by which a module is supposed to be put into stdlib, or fusion, or a separate nimble package

you are not alone; for me it's still very much an experiment, and some downsides are as follows:

on the flip side, the positive aspects about fusion would be:

dom96 commented 3 years ago

Is the cli module a separate nimble package, or does it come inside the nimble package?

Yeah, it's part of the nimble package. To be fair, now that Nimble supports multiple packages in the same repo, exposing the cli module as another package should be very trivial. I would prefer to see that as a PR rather than this :)