[1.x] Pass reload action for `watchOnTermination` callback #7838

Friendseeker · 2024-10-28T05:54:24Z

Issue

In watch mode, when a Reload action is triggered, sbt do not pass the Reload action to watchOnTermination callback (instead falls back to CancelWatch action)

Since onTerminationImpl (default implementation of watchOnTermination) does filtering based on type of action passed in, not passing Reload action cause unnecessary filtering resulting in #7017.

sbt/main/src/main/scala/sbt/nio/Watch.scala

Lines 554 to 575 in 2259598

    
             /** 
        
              * Default handler to transform the state when the watch terminates. When the [[Watch.Action]] 
        
              * is [[Reload]], the handler will prepend the original command (prefixed by ~) to the 
        
              * [[State.remainingCommands]] and then invoke the [[StateOps.reload]] method. When the 
        
              * [[Watch.Action]] is [[HandleError]], the handler returns the result of [[StateOps.fail]]. 
        
              * When the [[Watch.Action]] is [[Watch.Run]], we add the commands specified by 
        
              * [[Watch.Run.commands]] to the stat's remaining commands. Otherwise the original state is 
        
              * returned. 
        
              */ 
        
             private def onTerminationImpl( 
        
                 watchPrefixes: String* 
        
             ): (Action, String, Int, State) => State = { (action, command, count, state) => 
        
               val prefix = watchPrefixes.head 
        
               val rc = state.remainingCommands 
        
                 .filterNot(c => watchPrefixes.exists(c.commandLine.trim.startsWith)) 
        
               action match { 
        
                 case Run(commands) => state.copy(remainingCommands = commands ++ rc) 
        
                 case Reload => 
        
                   state.copy(remainingCommands = "reload".toExec :: s"$prefix $count $command".toExec :: rc) 
        
                 case _: HandleError => state.copy(remainingCommands = rc).fail 
        
                 case _              => state.copy(remainingCommands = rc) 
        
               }

Fix

#6870 introduces failAction as a mechanism to store & pass action to watchOnTermination callback. This PR employs same mechanism to pass watch action.

Closes #7017

eed3si9n

Thanks!

Set correct action for onTermination callback

6d4d216

Friendseeker requested a review from eed3si9n October 28, 2024 06:05

eed3si9n approved these changes Oct 28, 2024

View reviewed changes

eed3si9n merged commit 64f0acf into sbt:1.10.x Oct 28, 2024
10 checks passed

eed3si9n added this to the 1.10.4 milestone Oct 28, 2024

Friendseeker deleted the sbt-7017 branch October 28, 2024 07:17

Friendseeker mentioned this pull request Oct 29, 2024

[1.x] Add scripted test for #7838 #7844

Open

mrdziuban mentioned this pull request Oct 31, 2024

Triggered execution with multiple commands doesn't re-run the first command after an automatic reload #6984

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[1.x] Pass reload action for `watchOnTermination` callback #7838

[1.x] Pass reload action for `watchOnTermination` callback #7838

Uh oh!

Friendseeker commented Oct 28, 2024 •

edited

Loading

Uh oh!

eed3si9n left a comment

Uh oh!

Uh oh!

Uh oh!

	/**
	* Default handler to transform the state when the watch terminates. When the [[Watch.Action]]
	* is [[Reload]], the handler will prepend the original command (prefixed by ~) to the
	* [[State.remainingCommands]] and then invoke the [[StateOps.reload]] method. When the
	* [[Watch.Action]] is [[HandleError]], the handler returns the result of [[StateOps.fail]].
	* When the [[Watch.Action]] is [[Watch.Run]], we add the commands specified by
	* [[Watch.Run.commands]] to the stat's remaining commands. Otherwise the original state is
	* returned.
	*/
	private def onTerminationImpl(
	watchPrefixes: String*
	): (Action, String, Int, State) => State = { (action, command, count, state) =>
	val prefix = watchPrefixes.head
	val rc = state.remainingCommands
	.filterNot(c => watchPrefixes.exists(c.commandLine.trim.startsWith))
	action match {
	case Run(commands) => state.copy(remainingCommands = commands ++ rc)
	case Reload =>
	state.copy(remainingCommands = "reload".toExec :: s"$prefix $count $command".toExec :: rc)
	case _: HandleError => state.copy(remainingCommands = rc).fail
	case _ => state.copy(remainingCommands = rc)
	}

[1.x] Pass reload action for watchOnTermination callback #7838

[1.x] Pass reload action for watchOnTermination callback #7838

Uh oh!

Conversation

Friendseeker commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Fix

Uh oh!

eed3si9n left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[1.x] Pass reload action for `watchOnTermination` callback #7838

[1.x] Pass reload action for `watchOnTermination` callback #7838

Friendseeker commented Oct 28, 2024 •

edited

Loading