On 2007.10.23 11:29:34 +0300, "Valery V. Vorotyntsev" scribbled 0 lines:

On 10/19/07, gwern0@gmail.com wrote:

...

Fri Oct 19 14:09:00 EDT 2007 gwern0@gmail.com * Run.hs, ShellPrompt.sh: mv runInXTerm to ShellPrompt.hs

What is the purpose of this moving? IMHO, the function fits nicely in Run.hs.

IIRC, leaving runInXTerm in Run.hs causes a cyclical import - but I might have moved it because it made more sense to me in there.

...

Fri Oct 19 14:12:32 EDT 2007 gwern0@gmail.com * XSelection.hs: fmt imports and sigs

Fri Oct 19 14:12:55 EDT 2007 gwern0@gmail.com * SshPrompt.hs: fmt imports and update

Fri Oct 19 14:13:17 EDT 2007 gwern0@gmail.com * ShellPrompt.hs: fmt imports and update

The code looks somewhat polluted now. :) ``*Do not use explicit import lists*, except to resolve name clashes.'' - GHC Coding Style Guidelines, http://hackage.haskell.org/trac/ghc/wiki/Commentary/CodingStyle

-- WBR, vvv

Well, I'm not sure what's right for GHC's modules is right for XMonad's modules. I personally find they make things easier, but that may be just me. -- gwern fake E-Bomb NAAP FSF WORM SUW NMI Merlin charge CUN

On 2007.10.23 14:50:35 +0200, Christian Thiemann scribbled 0 lines:

Gwern Branwen wrote:

...

On 2007.10.23 11:29:34 +0300, "Valery V. Vorotyntsev" scribbled 0 lines:

...

On 10/19/07, gwern0@gmail.com wrote:

...

Fri Oct 19 14:09:00 EDT 2007 gwern0@gmail.com * Run.hs, ShellPrompt.sh: mv runInXTerm to ShellPrompt.hs What is the purpose of this moving? IMHO, the function fits nicely in Run.hs. IIRC, leaving runInXTerm in Run.hs causes a cyclical import - but I might have moved it because it made more sense to me in there.

The function was in a stand-alone module named RunInXTerm before the runXXX functions were collected into Run. So the situation for the function itself is unchanged. Probably the dependencies of the other runXXX-functions cause some cyclical imports, but Run.hs just imports XMonad from the core and nothing from the contrib, so I don't see where a problem might arise, unless XMonad.hs imports some contrib modules, which is not the case.

Conceptually I find it odd that I now need to import the whole ShellPrompt facility just to get a command that has nothing to do with ShellPrompt.

Greetings, Christian

Perhaps I don't understand your intent with runInXTerm then? My understanding was that it was supposed to take a string argument, pass it through the shell which would do whatever it specified, and then run the resultant command in an terminal specified by the shell (falling back to XTerm). This makes perfect sense to go into ShellPrompt to me - it's dealing with the shell, after all. Run.hs has various ways of running commands, ShellPrompt has various ways of running shell commands, and XSelection has various ways of dealing with the X selection. -- gwern Case Counterterrorism nonac SCOI SUBACS tekka Blacknet NRC SIGS JIC

On 2007.10.23 23:07:24 +0200, Christian Thiemann scribbled 0 lines:

Gwern Branwen wrote:

...

...

...

On 2007.10.23 11:29:34 +0300, "Valery V. Vorotyntsev" scribbled 0 lines:

...

On 10/19/07, gwern0@gmail.com wrote:

...

Fri Oct 19 14:09:00 EDT 2007 gwern0@gmail.com * Run.hs, ShellPrompt.sh: mv runInXTerm to ShellPrompt.hs What is the purpose of this moving? IMHO, the function fits nicely in Run.hs. IIRC, leaving runInXTerm in Run.hs causes a cyclical import - but I might have moved it because it made more sense to me in there. The function was in a stand-alone module named RunInXTerm before the runXXX functions were collected into Run. So the situation for the function itself is unchanged. Probably the dependencies of the other runXXX-functions cause some cyclical imports, but Run.hs just imports XMonad from the core and nothing from the contrib, so I don't see where a

Gwern Branwen wrote: problem might arise, unless XMonad.hs imports some contrib modules, which is not the case.

Conceptually I find it odd that I now need to import the whole ShellPrompt facility just to get a command that has nothing to do with ShellPrompt.

Greetings, Christian Perhaps I don't understand your intent with runInXTerm then? My understanding was that it was supposed to take a string argument, pass it

On 2007.10.23 14:50:35 +0200, Christian Thiemann scribbled 0 lines: through the shell which would do whatever it specified, and then run the resultant command in an terminal specified by the shell (falling back to XTerm). This makes perfect sense to go into ShellPrompt to me - it's dealing with the shell, after all. Run.hs has various ways of running commands, ShellPrompt has various ways of running shell commands, and XSelection has various ways of dealing with the X selection.

ShellPrompt is providing a graphical prompt asking for a command to execute (not in a terminal - runInXTerm was introduced by Andrea in RunInXTerm.hs for import in SshPrompt.hs to bring up an ssh in a shell). But you don't need the graphical ShellPrompt or the SshPrompt for running something in XTerm. The probably more convincing examples are runProcessWithInput and runProcessWithInputAndWait: these were originally defined in Dmenu.hs and Dzen.hs - because they were handy functions to bring up a dmenu or dzen instance - and some other contribs then included Dmenu or Dzen just to get these functions, without using the dmenu or dzen related stuff. That seemed odd to me. Similarly, SshPrompt relies on being able to run something in a terminal (thus it should import some module that provides this functionality) but runInXTerm does not need the graphical SshPrompt (or ShellPrompt), thus I think one should not need to include ShellPrompt to be able to run something in a terminal.

Greetings, Christian

-- Christian Thiemann

Alright, I think I'm understanding this better. I'm not too thrilled about runInXTerm as it currently exists. The method is cute, certainly, seeing whether an environmental variable exists and if not falling back to XTerm, but it grates on me. Do you think a new definition would be better, along the lines of runInTerm :: String -> String -> X () runInTerm term command = safeSpawn term ("-e " ++ command) This definition, I think, would make sense to move back into Run.hs, resolving most of your complaint, and it also has other benefits: this way, it's really obvious what the function is doing; the user can easily specify which terminal she wants to use *in* XMonad and not Bash or Zsh or whatever; this would also be nice since that's a common idiom in my Config.hs. What does everyone think? -- gwern Guantanamo CIA secret MI-17 schloss DEADBEEF MJ-12 SADMS dictionary 82

On 2007.10.23 15:36:12 +0200, Andrea Rossato scribbled 0 lines:

On Tue, Oct 23, 2007 at 08:20:50AM -0400, Gwern Branwen wrote:

...

On 2007.10.23 11:29:34 +0300, "Valery V. Vorotyntsev" scribbled 0 lines:

...

On 10/19/07, gwern0@gmail.com wrote:

...

Fri Oct 19 14:09:00 EDT 2007 gwern0@gmail.com * Run.hs, ShellPrompt.sh: mv runInXTerm to ShellPrompt.hs

What is the purpose of this moving? IMHO, the function fits nicely in Run.hs.

IIRC, leaving runInXTerm in Run.hs causes a cyclical import - but I might have moved it because it made more sense to me in there.

...

...

Fri Oct 19 14:12:32 EDT 2007 gwern0@gmail.com * XSelection.hs: fmt imports and sigs

Fri Oct 19 14:12:55 EDT 2007 gwern0@gmail.com * SshPrompt.hs: fmt imports and update

Fri Oct 19 14:13:17 EDT 2007 gwern0@gmail.com * ShellPrompt.hs: fmt imports and update

The code looks somewhat polluted now. :) ``*Do not use explicit import lists*, except to resolve name clashes.'' - GHC Coding Style Guidelines, http://hackage.haskell.org/trac/ghc/wiki/Commentary/CodingStyle

-- WBR, vvv

Well, I'm not sure what's right for GHC's modules is right for XMonad's modules. I personally find they make things easier, but that may be just me.

I must confess I don't think I like these patches very much. Lately I was very busy with the starting of the new semester and didn't have time to review the code carefully (I still have to pull them) but they add functionalities that seem not to be related to a shell prompt at first sight, and should probably belong to a different module.

Regarding the style: I hope that Don will come up with some coding convention I remember he was talking about.

For the time being, and for the code I maintain, I believe that patches should not change the coding style of a module to adapt it to every contributor's own feelings. This would lead to some confusion. So, if the coding style of a module is not a specific issue itself, I would leave it unchanged, or let the original author decide. And so I think I'll bring that module back to its pristine status.

Yes, this struck me as possible, which is why the formatting was (generally) split out to their own patches. I didn't separate them as cleanly as I could have, perhaps, but I didn't *really* expect anyone to take exception.

I'll probably going to add a different version of the main function, because I think it could be useful a different way of handling completion since the latest changes were not welcome but everyone - the way you use the prompt may required "unoptimized" code for efficiency.

The other prompts - but I will first review the problem more carefully - can be moved to a new prompt module, connected to XSelection if I understand correctly the issue.

The prompts which use XSelection's getSelection are already in XSelection - unsafePromptSelection, and safePromptSelection. They do rely on the safeSpawn and unsafeSpawn in Run.hs however, and have nothing to do with ShellPrompt.

Cheers, Andrea

-- gwern fake E-Bomb NAAP FSF WORM SUW NMI Merlin charge CUN

On Tue, Oct 23, 2007 at 11:29:34AM +0300, Valery V. Vorotyntsev wrote:

On 10/19/07, gwern0@gmail.com wrote:

...

Fri Oct 19 14:13:17 EDT 2007 gwern0@gmail.com * ShellPrompt.hs: fmt imports and update

The code looks somewhat polluted now. :) ``*Do not use explicit import lists*, except to resolve name clashes.'' - GHC Coding Style Guidelines, http://hackage.haskell.org/trac/ghc/wiki/Commentary/CodingStyle

There's a difference between coding a compiler and coding anything else. SPJ's recommendation for libraries is to *always* use explicit import lists, as this gives maximum forward-compatibility, since otherwise your code could break when a new function is added to the imported module, due to name clashes. There was discussion recently (on haskell-cafe) of adding a warning for imports without an import list. I hope it gets added, as it'll make it easier to enforce darcs' coding guidelines. :) I also prefer explicit import lists, because they make code easier to read. Rather than searching online for the docs to a half dozen modules, trying to find out what a function means, you can go directly to the module that exports that function. -- David Roundy Department of Physics Oregon State University

On Tue, Oct 23, 2007 at 05:25:46PM +0300, Valery V. Vorotyntsev wrote:

On 10/23/07, David Roundy wrote:

...

There's a difference between coding a compiler and coding anything else. SPJ's recommendation for libraries is to *always* use explicit import lists, as this gives maximum forward-compatibility, since otherwise your code could break when a new function is added to the imported module, due to name clashes. There was discussion recently (on haskell-cafe) of adding a warning for imports without an import list. I hope it gets added, as it'll make it easier to enforce darcs' coding guidelines. :)

Could you refer me to the darcs guidelines? :) I've googled nothing.

I'm afraid we haven't written them up. But if you were to read the code, you'd find that it's pretty clear, and it's definitely been discussed on the mailing list. -- David Roundy Department of Physics Oregon State University